From 026d378c0e4af9188f4a13da2ac0dc4ce4a6d5df Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 11 Aug 2025 23:26:13 +0200 Subject: [PATCH] fix some last clap errors mgmt --- src/uucore/src/lib/lib.rs | 2 +- src/uucore/src/lib/mods/clap_localization.rs | 69 ++++++++++++++------ tests/by-util/test_base32.rs | 5 +- tests/by-util/test_base64.rs | 3 +- tests/by-util/test_du.rs | 4 +- tests/by-util/test_sort.rs | 13 +--- tests/by-util/test_split.rs | 5 +- 7 files changed, 61 insertions(+), 40 deletions(-) diff --git a/src/uucore/src/lib/lib.rs b/src/uucore/src/lib/lib.rs index 6b1419e62..af0e039d0 100644 --- a/src/uucore/src/lib/lib.rs +++ b/src/uucore/src/lib/lib.rs @@ -5,7 +5,7 @@ //! library ~ (core/bundler file) // #![deny(missing_docs)] //TODO: enable this // -// spell-checker:ignore sigaction SIGBUS SIGSEGV extendedbigdecimal +// spell-checker:ignore sigaction SIGBUS SIGSEGV extendedbigdecimal myutil // * feature-gated external crates (re-shared as public internal modules) #[cfg(feature = "libc")] diff --git a/src/uucore/src/lib/mods/clap_localization.rs b/src/uucore/src/lib/mods/clap_localization.rs index d40a54a6f..a8a783ad6 100644 --- a/src/uucore/src/lib/mods/clap_localization.rs +++ b/src/uucore/src/lib/mods/clap_localization.rs @@ -212,30 +212,61 @@ pub fn handle_clap_error_with_exit_code(err: Error, util_name: &str, exit_code: let option = arg.to_string(); let value = value.to_string(); - // Get localized error word - let error_word = translate!("common-error"); - let colored_error_word = maybe_colorize(&error_word, Color::Red); + // Check if this is actually a missing value (empty string) + if value.is_empty() { + // This is the case where no value was provided for an option that requires one + eprintln!( + "error: a value is required for '{}' but none was supplied", + option + ); + eprintln!(); + eprintln!("For more information, try '--help'."); + std::process::exit(1); + } else { + // Get localized error word + let error_word = translate!("common-error"); + let colored_error_word = maybe_colorize(&error_word, Color::Red); - // Apply color to value and option if colors are enabled - let colored_value = maybe_colorize(&value, Color::Yellow); - let colored_option = maybe_colorize(&option, Color::Green); + // Apply color to value and option if colors are enabled + let colored_value = maybe_colorize(&value, Color::Yellow); + let colored_option = maybe_colorize(&option, Color::Green); - // Print localized error message - let error_msg = translate!( - "clap-error-invalid-value", - "error_word" => colored_error_word, - "value" => colored_value, - "option" => colored_option - ); - eprintln!("{error_msg}"); - - // For ValueValidation errors, include the validation error details - if matches!(kind, ErrorKind::ValueValidation) { - if let Some(source) = err.source() { - eprintln!(" {}", source); + // For ValueValidation errors, include the validation error in the message + if matches!(kind, ErrorKind::ValueValidation) { + if let Some(source) = err.source() { + // Print error with validation detail on same line + let error_msg = translate!( + "clap-error-invalid-value", + "error_word" => colored_error_word, + "value" => colored_value, + "option" => colored_option + ); + eprintln!("{error_msg}: {}", source); + } else { + // Print localized error message + let error_msg = translate!( + "clap-error-invalid-value", + "error_word" => colored_error_word, + "value" => colored_value, + "option" => colored_option + ); + eprintln!("{error_msg}"); + } + } else { + // Print localized error message + let error_msg = translate!( + "clap-error-invalid-value", + "error_word" => colored_error_word, + "value" => colored_value, + "option" => colored_option + ); + eprintln!("{error_msg}"); } } + // For ValueValidation errors, include the validation error details + // Note: We don't print these separately anymore as they're part of the main message + // Show possible values if available (for InvalidValue errors) if matches!(kind, ErrorKind::InvalidValue) { if let Some(valid_values) = err.get(ContextKind::ValidValue) { diff --git a/tests/by-util/test_base32.rs b/tests/by-util/test_base32.rs index 8bdf19ed0..252256668 100644 --- a/tests/by-util/test_base32.rs +++ b/tests/by-util/test_base32.rs @@ -112,13 +112,12 @@ fn test_wrap() { #[test] fn test_wrap_no_arg() { - let expected_stderr = "a value is required for '--wrap ' but none was supplied"; - for wrap_param in ["-w", "--wrap"] { new_ucmd!() .arg(wrap_param) .fails() - .stderr_contains(expected_stderr) + .stderr_contains("error: a value is required for '--wrap ' but none was supplied") + .stderr_contains("For more information, try '--help'.") .no_stdout(); } } diff --git a/tests/by-util/test_base64.rs b/tests/by-util/test_base64.rs index 2feb6ceff..ad0d1c2b1 100644 --- a/tests/by-util/test_base64.rs +++ b/tests/by-util/test_base64.rs @@ -147,7 +147,8 @@ fn test_wrap_no_arg() { new_ucmd!() .arg(wrap_param) .fails() - .stderr_contains("a value is required for '--wrap ' but none was supplied") + .stderr_contains("error: a value is required for '--wrap ' but none was supplied") + .stderr_contains("For more information, try '--help'.") .no_stdout(); } } diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index ba64152e7..86c358724 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -862,7 +862,9 @@ fn test_du_threshold_error_handling() { new_ucmd!() .arg("--threshold") .fails() - .stderr_contains("a value is required for '--threshold ' but none was supplied") + .stderr_contains( + "error: a value is required for '--threshold ' but none was supplied", + ) .stderr_contains("For more information, try '--help'."); } diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 07ce267f2..44c694860 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -1684,19 +1684,8 @@ fn test_clap_localization_missing_required_argument() { .fails(); let stderr_en = result_en.stderr_str(); - assert!(stderr_en.contains("error:")); + assert!(stderr_en.contains(" a value is required for '--key ' but none was supplied")); assert!(stderr_en.contains("-k")); - - // Test in French - let result_fr = new_ucmd!() - .env("LANG", "fr_FR.UTF-8") - .env("LC_ALL", "fr_FR.UTF-8") - .arg("-k") - .fails(); - - let stderr_fr = result_fr.stderr_str(); - // The main error message should contain French "erreur" - assert!(stderr_fr.contains("error:") || stderr_fr.contains("erreur")); } #[test] diff --git a/tests/by-util/test_split.rs b/tests/by-util/test_split.rs index a91ffc8ff..34b24d84d 100644 --- a/tests/by-util/test_split.rs +++ b/tests/by-util/test_split.rs @@ -1935,9 +1935,8 @@ fn test_split_separator_no_value() { .ignore_stdin_write_error() .pipe_in("a\n") .fails() - .stderr_contains( - "error: a value is required for '--separator ' but none was supplied", - ); + .stderr_contains("error: a value is required for '--separator ' but none was supplied") + .stderr_contains("For more information, try '--help'."); } #[test]