From 79a6c78032f93e448287740aaefa959570351391 Mon Sep 17 00:00:00 2001 From: Bahex <17417311+Bahex@users.noreply.github.com> Date: Tue, 12 Aug 2025 23:25:23 +0300 Subject: [PATCH] implement `FromValue` for `std::time::Duration` and refactor relevant commands to utilize that (#16414) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # 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::shell::type_mismatch × Type mismatch. ╭─[entry #40:1:9] 1 │ watch . --debounce (1 | $in) {|| } · ──────────┬───────── · ╰── Debounce duration must be a duration ╰──── ``` - After ``` Error: nu::shell::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::shell::type_mismatch × Type mismatch. ╭─[entry #41:1:9] 1 │ watch . --debounce -100ms {|| } · ────────┬──────── · ╰── Debounce duration is invalid ╰──── ``` - After ``` Error: nu::shell::needs_positive_value × Negative value passed when positive one is required ╭─[entry #4:1:9] 1 │ watch . --debounce -100ms {|| } · ────────┬──────── · ╰── use a positive value ╰──── ``` --- .../nu-command/src/experimental/job_recv.rs | 4 +- crates/nu-command/src/filesystem/watch.rs | 60 ++++++++----------- crates/nu-protocol/src/value/from_value.rs | 23 +++++++ 3 files changed, 50 insertions(+), 37 deletions(-) diff --git a/crates/nu-command/src/experimental/job_recv.rs b/crates/nu-command/src/experimental/job_recv.rs index a3a250b275..aa13700780 100644 --- a/crates/nu-command/src/experimental/job_recv.rs +++ b/crates/nu-command/src/experimental/job_recv.rs @@ -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 duration: Option = call.get_flag(engine_state, stack, "timeout")?; - - let timeout = duration.map(|it| Duration::from_nanos(it as u64)); + let timeout: Option = call.get_flag(engine_state, stack, "timeout")?; let mut mailbox = engine_state .current_job diff --git a/crates/nu-command/src/filesystem/watch.rs b/crates/nu-command/src/filesystem/watch.rs index 9dc759ea21..5599c39c51 100644 --- a/crates/nu-command/src/filesystem/watch.rs +++ b/crates/nu-command/src/filesystem/watch.rs @@ -119,42 +119,11 @@ impl Command for Watch { let debounce_duration_flag_ms: Option> = call.get_flag(engine_state, stack, "debounce-ms")?; - let debounce_duration_flag: Option> = + let debounce_duration_flag: Option> = call.get_flag(engine_state, stack, "debounce")?; - let debounce_duration: Duration = match (debounce_duration_flag, debounce_duration_flag_ms) - { - (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 debounce_duration: Duration = + resolve_duration_arguments(debounce_duration_flag_ms, debounce_duration_flag)?; let glob_flag: Option> = call.get_flag(engine_state, stack, "glob")?; let glob_pattern = match glob_flag { @@ -353,3 +322,26 @@ impl Command for Watch { ] } } + +fn resolve_duration_arguments( + debounce_duration_flag_ms: Option>, + debounce_duration_flag: Option>, +) -> Result { + 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), + } +} diff --git a/crates/nu-protocol/src/value/from_value.rs b/crates/nu-protocol/src/value/from_value.rs index 708f1f69c5..1822abbcda 100644 --- a/crates/nu-protocol/src/value/from_value.rs +++ b/crates/nu-protocol/src/value/from_value.rs @@ -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 { + 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 FromValue for NonZero as NonZero requires an unstable trait // As a result, we use this macro to implement FromValue for each NonZero type.