implement FromValue for std::time::Duration and refactor relevant commands to utilize that (#16414)

# Description

- Implemented `FromValue` for `std::time::Duration`.
- It only converts positive `Value::Duration` values, negative ones
raise `ShellError::NeedsPositiveValue`.
- Refactor `job recv` and `watch` commands to use this implementation
rather than handling it ad-hoc.
- Simplified `watch`'s `debounce` & `debounce-ms` and factored it to a
function. Should make removing `debounce-ms` after its deprecation
period ends.
- `job recv` previously used a numeric cast (`i64 as u64`) which would
result in extremely long duration values rather than raising an error
when negative duration arguments were given.

# User-Facing Changes

Changes in error messages:
- Providing the wrong type (bypassing parse time type checking):
  - Before
    ```
    Error: nu:🐚:type_mismatch

      × Type mismatch.
       ╭─[entry #40:1:9]
     1 │ watch . --debounce (1 | $in) {|| }
       ·         ──────────┬─────────
       ·                   ╰── Debounce duration must be a duration
       ╰────
    ```
  - After
    ```
    Error: nu:🐚:cant_convert

      × Can't convert to duration.
       ╭─[entry #2:1:9]
     1 │ watch . --debounce (1 | $in) {|| }
       ·         ──────────┬─────────
       ·                   ╰── can't convert int to duration
       ╰────
    ```
- Providing a negative duration value:
  - Before
    ```
    Error: nu:🐚:type_mismatch

      × Type mismatch.
       ╭─[entry #41:1:9]
     1 │ watch . --debounce -100ms {|| }
       ·         ────────┬────────
       ·                 ╰── Debounce duration is invalid
       ╰────
    ```
  - After
    ```
    Error: nu:🐚:needs_positive_value

      × Negative value passed when positive one is required
       ╭─[entry #4:1:9]
     1 │ watch . --debounce -100ms {|| }
       ·         ────────┬────────
       ·                 ╰── use a positive value
       ╰────
    ```
This commit is contained in:
Bahex
2025-08-12 23:25:23 +03:00
committed by GitHub
parent 5478bdff0e
commit 79a6c78032
3 changed files with 50 additions and 37 deletions

View File

@ -78,9 +78,7 @@ in no particular order, regardless of the specified timeout parameter.
let tag = tag_arg.map(|it| it.item as FilterTag); let tag = tag_arg.map(|it| it.item as FilterTag);
let duration: Option<i64> = call.get_flag(engine_state, stack, "timeout")?; let timeout: Option<Duration> = call.get_flag(engine_state, stack, "timeout")?;
let timeout = duration.map(|it| Duration::from_nanos(it as u64));
let mut mailbox = engine_state let mut mailbox = engine_state
.current_job .current_job

View File

@ -119,42 +119,11 @@ impl Command for Watch {
let debounce_duration_flag_ms: Option<Spanned<i64>> = let debounce_duration_flag_ms: Option<Spanned<i64>> =
call.get_flag(engine_state, stack, "debounce-ms")?; call.get_flag(engine_state, stack, "debounce-ms")?;
let debounce_duration_flag: Option<Spanned<Value>> = let debounce_duration_flag: Option<Spanned<Duration>> =
call.get_flag(engine_state, stack, "debounce")?; call.get_flag(engine_state, stack, "debounce")?;
let debounce_duration: Duration = match (debounce_duration_flag, debounce_duration_flag_ms) let debounce_duration: Duration =
{ resolve_duration_arguments(debounce_duration_flag_ms, debounce_duration_flag)?;
(None, None) => DEFAULT_WATCH_DEBOUNCE_DURATION,
(Some(l), Some(r)) => {
return Err(ShellError::IncompatibleParameters {
left_message: "Here".to_string(),
left_span: l.span,
right_message: "and here".to_string(),
right_span: r.span,
});
}
(None, Some(val)) => match u64::try_from(val.item) {
Ok(v) => Duration::from_millis(v),
Err(_) => {
return Err(ShellError::TypeMismatch {
err_message: "Debounce duration is invalid".to_string(),
span: val.span,
});
}
},
(Some(v), None) => {
let Value::Duration { val, .. } = v.item else {
return Err(ShellError::TypeMismatch {
err_message: "Debounce duration must be a duration".to_string(),
span: v.item.span(),
});
};
Duration::from_nanos(u64::try_from(val).map_err(|_| ShellError::TypeMismatch {
err_message: "Debounce duration is invalid".to_string(),
span: v.item.span(),
})?)
}
};
let glob_flag: Option<Spanned<String>> = call.get_flag(engine_state, stack, "glob")?; let glob_flag: Option<Spanned<String>> = call.get_flag(engine_state, stack, "glob")?;
let glob_pattern = match glob_flag { let glob_pattern = match glob_flag {
@ -353,3 +322,26 @@ impl Command for Watch {
] ]
} }
} }
fn resolve_duration_arguments(
debounce_duration_flag_ms: Option<Spanned<i64>>,
debounce_duration_flag: Option<Spanned<Duration>>,
) -> Result<Duration, ShellError> {
match (debounce_duration_flag, debounce_duration_flag_ms) {
(None, None) => Ok(DEFAULT_WATCH_DEBOUNCE_DURATION),
(Some(l), Some(r)) => Err(ShellError::IncompatibleParameters {
left_message: "Here".to_string(),
left_span: l.span,
right_message: "and here".to_string(),
right_span: r.span,
}),
(None, Some(val)) => match u64::try_from(val.item) {
Ok(v) => Ok(Duration::from_millis(v)),
Err(_) => Err(ShellError::TypeMismatch {
err_message: "Debounce duration is invalid".to_string(),
span: val.span,
}),
},
(Some(v), None) => Ok(v.item),
}
}

View File

@ -275,6 +275,29 @@ impl FromValue for i64 {
} }
} }
/// This implementation supports **positive** durations only.
impl FromValue for std::time::Duration {
fn from_value(v: Value) -> Result<Self, ShellError> {
match v {
Value::Duration { val, .. } => {
let nanos = u64::try_from(val)
.map_err(|_| ShellError::NeedsPositiveValue { span: v.span() })?;
Ok(Self::from_nanos(nanos))
}
v => Err(ShellError::CantConvert {
to_type: Self::expected_type().to_string(),
from_type: v.get_type().to_string(),
span: v.span(),
help: None,
}),
}
}
fn expected_type() -> Type {
Type::Duration
}
}
// //
// We can not use impl<T: FromValue> FromValue for NonZero<T> as NonZero requires an unstable trait // We can not use impl<T: FromValue> FromValue for NonZero<T> as NonZero requires an unstable trait
// As a result, we use this macro to implement FromValue for each NonZero type. // As a result, we use this macro to implement FromValue for each NonZero type.