From 946aab37ede4b050c6ad287aa673966d51bfddaa Mon Sep 17 00:00:00 2001 From: Joining7943 <111500881+Joining7943@users.noreply.github.com> Date: Tue, 28 Feb 2023 07:36:04 +0100 Subject: [PATCH 1/3] Cargo: Bump fundu version v0.3.0 -> v0.4.2. Adjust test in test_tail. --- .vscode/cspell.dictionaries/workspace.wordlist.txt | 1 + Cargo.lock | 4 ++-- Cargo.toml | 2 +- tests/by-util/test_tail.rs | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.vscode/cspell.dictionaries/workspace.wordlist.txt b/.vscode/cspell.dictionaries/workspace.wordlist.txt index d6e91dca5..6d6533bcf 100644 --- a/.vscode/cspell.dictionaries/workspace.wordlist.txt +++ b/.vscode/cspell.dictionaries/workspace.wordlist.txt @@ -20,6 +20,7 @@ exacl filetime formatteriteminfo fsext +fundu getopts getrandom globset diff --git a/Cargo.lock b/Cargo.lock index 4d53a4c2e..ca348f194 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -880,9 +880,9 @@ dependencies = [ [[package]] name = "fundu" -version = "0.3.0" +version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "925250bc259498d4008ee072bf16586083ab2c491aa4b06b3c4d0a6556cebd74" +checksum = "da58c38fe7b706cead98429d8a8535261addbe55fd531c7d7c7d770346464010" [[package]] name = "futures" diff --git a/Cargo.toml b/Cargo.toml index be545c860..1fe04f34c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -282,7 +282,7 @@ filetime = "0.2" fnv = "1.0.7" fs_extra = "1.1.0" fts-sys = "0.2" -fundu = "0.3.0" +fundu = "0.4.2" gcd = "2.2" glob = "0.3.0" half = "2.1" diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index eb18e858f..93ca1046d 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -4459,7 +4459,7 @@ fn test_follow_when_files_are_pointing_to_same_relative_file_and_file_stays_same } #[rstest] -#[case::exponent_exceed_float_max("1.0e2048")] +#[case::exponent_exceed_float_max("1.0e100000")] #[case::underscore_delimiter("1_000")] #[case::only_point(".")] #[case::space_in_primes("' '")] From 3a28b616d998cf2e1917ebe947893b37dabca529 Mon Sep 17 00:00:00 2001 From: Joining7943 <111500881+Joining7943@users.noreply.github.com> Date: Tue, 28 Feb 2023 07:36:22 +0100 Subject: [PATCH 2/3] sleep: Use fundu instead of uucore::parse_time::from_str --- Cargo.lock | 1 + src/uu/sleep/Cargo.toml | 1 + src/uu/sleep/src/sleep.rs | 7 ++++++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index ca348f194..98b5cb973 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3041,6 +3041,7 @@ name = "uu_sleep" version = "0.0.17" dependencies = [ "clap", + "fundu", "uucore", ] diff --git a/src/uu/sleep/Cargo.toml b/src/uu/sleep/Cargo.toml index 048f551c1..aa4bf0664 100644 --- a/src/uu/sleep/Cargo.toml +++ b/src/uu/sleep/Cargo.toml @@ -16,6 +16,7 @@ path = "src/sleep.rs" [dependencies] clap = { workspace=true } +fundu = { workspace=true } uucore = { workspace=true } [[bin]] diff --git a/src/uu/sleep/src/sleep.rs b/src/uu/sleep/src/sleep.rs index 4751d89b2..af6a0f22e 100644 --- a/src/uu/sleep/src/sleep.rs +++ b/src/uu/sleep/src/sleep.rs @@ -14,6 +14,7 @@ use uucore::{ }; use clap::{crate_version, Arg, ArgAction, Command}; +use fundu::{self, DurationParser}; static ABOUT: &str = help_about!("sleep.md"); const USAGE: &str = help_usage!("sleep.md"); @@ -61,10 +62,14 @@ pub fn uu_app() -> Command { fn sleep(args: &[&str]) -> UResult<()> { let mut arg_error = false; + + use fundu::TimeUnit::*; + let parser = DurationParser::with_time_units(&[Second, Minute, Hour, Day]); + let sleep_dur = args .iter() .filter_map(|input| { - uucore::parse_time::from_str(input.trim()).ok().or_else(|| { + parser.parse(input.trim()).ok().or_else(|| { arg_error = true; show_error!("invalid time interval '{input}'"); None From 5fb091a4fb14eed29894de19ab62212941945bb6 Mon Sep 17 00:00:00 2001 From: Joining7943 <111500881+Joining7943@users.noreply.github.com> Date: Wed, 1 Mar 2023 07:35:53 +0100 Subject: [PATCH 3/3] sleep: Use fundu error types to improve error messages --- src/uu/sleep/src/sleep.rs | 26 +++++++++++++++++++++----- tests/by-util/test_sleep.rs | 16 +++++++++------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/uu/sleep/src/sleep.rs b/src/uu/sleep/src/sleep.rs index af6a0f22e..009986095 100644 --- a/src/uu/sleep/src/sleep.rs +++ b/src/uu/sleep/src/sleep.rs @@ -14,7 +14,7 @@ use uucore::{ }; use clap::{crate_version, Arg, ArgAction, Command}; -use fundu::{self, DurationParser}; +use fundu::{self, DurationParser, ParseError}; static ABOUT: &str = help_about!("sleep.md"); const USAGE: &str = help_usage!("sleep.md"); @@ -68,12 +68,28 @@ fn sleep(args: &[&str]) -> UResult<()> { let sleep_dur = args .iter() - .filter_map(|input| { - parser.parse(input.trim()).ok().or_else(|| { + .filter_map(|input| match parser.parse(input.trim()) { + Ok(duration) => Some(duration), + Err(error) => { arg_error = true; - show_error!("invalid time interval '{input}'"); + + let reason = match error { + ParseError::Empty if input.is_empty() => "Input was empty".to_string(), + ParseError::Empty => "Found only whitespace in input".to_string(), + ParseError::Syntax(pos, description) + | ParseError::TimeUnit(pos, description) => { + format!("{description} at position {}", pos.saturating_add(1)) + } + ParseError::NegativeExponentOverflow | ParseError::PositiveExponentOverflow => { + "Exponent was out of bounds".to_string() + } + ParseError::NegativeNumber => "Number was negative".to_string(), + error => error.to_string(), + }; + show_error!("invalid time interval '{input}': {reason}"); + None - }) + } }) .fold(Duration::ZERO, |acc, n| acc.saturating_add(n)); diff --git a/tests/by-util/test_sleep.rs b/tests/by-util/test_sleep.rs index c525cc0d2..8bb4b11ea 100644 --- a/tests/by-util/test_sleep.rs +++ b/tests/by-util/test_sleep.rs @@ -10,11 +10,11 @@ fn test_invalid_time_interval() { new_ucmd!() .arg("xyz") .fails() - .usage_error("invalid time interval 'xyz'"); + .usage_error("invalid time interval 'xyz': Invalid character: 'x' at position 1"); new_ucmd!() .args(&["--", "-1"]) .fails() - .usage_error("invalid time interval '-1'"); + .usage_error("invalid time interval '-1': Number was negative"); } #[test] @@ -204,14 +204,16 @@ fn test_sleep_when_input_has_only_whitespace_then_error(#[case] input: &str) { .arg(input) .timeout(Duration::from_secs(10)) .fails() - .usage_error(format!("invalid time interval '{input}'")); + .usage_error(format!( + "invalid time interval '{input}': Found only whitespace in input" + )); } #[test] fn test_sleep_when_multiple_input_some_with_error_then_shows_all_errors() { - let expected = "invalid time interval 'abc'\n\ - sleep: invalid time interval '1years'\n\ - sleep: invalid time interval ' '"; + let expected = "invalid time interval 'abc': Invalid character: 'a' at position 1\n\ + sleep: invalid time interval '1years': Invalid time unit: 'years' at position 2\n\ + sleep: invalid time interval ' ': Found only whitespace in input"; // Even if one of the arguments is valid, but the rest isn't, we should still fail and exit early. // So, the timeout of 10 seconds ensures we haven't executed `thread::sleep` with the only valid @@ -228,5 +230,5 @@ fn test_negative_interval() { new_ucmd!() .args(&["--", "-1"]) .fails() - .usage_error("invalid time interval '-1'"); + .usage_error("invalid time interval '-1': Number was negative"); }