diff --git a/src/uu/chmod/locales/en-US.ftl b/src/uu/chmod/locales/en-US.ftl index 2f3f6748461..0df78e83b01 100644 --- a/src/uu/chmod/locales/en-US.ftl +++ b/src/uu/chmod/locales/en-US.ftl @@ -15,6 +15,7 @@ chmod-error-permission-denied = cannot access {$file}: Permission denied chmod-error-new-permissions = {$file}: new permissions are {$actual}, not {$expected} chmod-error-changing-permissions = changing permissions of {$file}: {$err} chmod-error-missing-operand = missing operand +chmod-error-invalid-mode = invalid mode: '{$mode}' # Help messages chmod-help-print-help = Print help information. diff --git a/src/uu/chmod/locales/fr-FR.ftl b/src/uu/chmod/locales/fr-FR.ftl index e8116f08f9c..aa7580607c9 100644 --- a/src/uu/chmod/locales/fr-FR.ftl +++ b/src/uu/chmod/locales/fr-FR.ftl @@ -27,6 +27,7 @@ chmod-error-permission-denied = impossible d'accéder à {$file} : Permission re chmod-error-new-permissions = {$file} : les nouvelles permissions sont {$actual}, pas {$expected} chmod-error-changing-permissions = changement des permissions de {$file} : {$err} chmod-error-missing-operand = opérande manquant +chmod-error-invalid-mode = mode invalide : '{$mode}' # Messages verbeux/de statut chmod-verbose-failed-dangling = échec du changement de mode de {$file} de 0000 (---------) vers 1500 (r-x-----T) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 6875ac66b84..f90631cb68a 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -13,9 +13,7 @@ use std::os::unix::fs::{MetadataExt, PermissionsExt}; use std::path::{Path, PathBuf}; use thiserror::Error; use uucore::display::Quotable; -use uucore::error::{ - ExitCode, UError, UResult, USimpleError, UUsageError, set_exit_code, strip_errno, -}; +use uucore::error::{ExitCode, UError, UResult, UUsageError, set_exit_code, strip_errno}; use uucore::fs::{FileInformation, display_permissions_unix}; use uucore::mode; use uucore::perms::{TraverseSymlinks, configure_symlink_and_recursion}; @@ -349,15 +347,24 @@ impl Chmoder { if self.quiet { return Err(ExitCode::new(1)); } + // GNU always names the whole mode operand, not the + // clause that broke, and always with this one + // message regardless of what specifically went + // wrong -- the more specific `error` still supplies + // the caret diagram's label when one is rendered. + let message = translate!( + "chmod-error-invalid-mode", + "mode" => cmode_unwrapped.clone() + ); if let Some(args) = &self.args && let Some((index, operand, offset)) = self.locate_clause(args, &cmode_unwrapped, clause_start) - && error.render_at(args, index, &operand, offset, &error.to_string()) + && error.render_at(args, index, &operand, offset, &message) { // The diagnostic is already on stderr; exit quietly. return Err(ExitCode::new(1)); } - return Err(USimpleError::new(1, error.to_string())); + return Err(UUsageError::new(1, message)); } } } diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 8dfeccb4c94..3a17fd87937 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -131,19 +131,18 @@ impl ModeError { /// The caret label for this error, translated, and the advice that goes /// under it. /// - /// Labelled only where a label would add to the message, per the - /// convention in [`crate::diagnostics`]. + /// The headline callers show is the same for every `ModeError` — GNU + /// itself only ever says "invalid mode: 'WHOLE_MODE'" — so the detail + /// that used to live there moves down to the label instead. fn describe(&self) -> (Option, Option) { let label = match self.kind { - // The message already names the expected operators. - ModeErrorKind::InvalidOperator => None, - ModeErrorKind::MissingOperator => Some("mode-diag-label-missing-operator"), - ModeErrorKind::InvalidNumber => Some("mode-diag-label-invalid-number"), + // `self.message` already names the operator that was expected + // and the one that was found instead. + ModeErrorKind::InvalidOperator => Some(self.message.clone()), + ModeErrorKind::MissingOperator => Some(translate!("mode-diag-label-missing-operator")), + ModeErrorKind::InvalidNumber => Some(translate!("mode-diag-label-invalid-number")), }; - ( - label.map(|label| translate!(label)), - Some(translate!("mode-diag-help-syntax")), - ) + (label, Some(translate!("mode-diag-help-syntax"))) } /// Where this error sits inside the whole mode, given where the clause it diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index ca13fdef473..30d7f47904f 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -285,6 +285,26 @@ fn test_chmod_error_permissions() { ); } +/// GNU says the same thing, "invalid mode: 'WHOLE_MODE'", for every way a +/// mode can be malformed -- a non-octal digit, a value too large, a missing +/// operator, or an unrecognized one -- so uutils does too, rather than +/// leaking the reason as a distinct message shaped like whatever failed to +/// parse it internally (a raw `ParseIntError`, for one). +#[test] +fn test_invalid_mode_names_the_whole_operand() { + let scenario = TestScenario::new(util_name!()); + let at = &scenario.fixtures; + at.touch("file"); + + for mode in ["999", "", "a", "u?rwx", "u+rwx,z+r", "12x"] { + scenario + .ucmd() + .args(&[mode, "file"]) + .fails_with_code(1) + .usage_error(format!("invalid mode: '{mode}'")); + } +} + #[test] fn test_chmod_permissions_too_large() { let scenario = TestScenario::new(util_name!()); @@ -296,19 +316,13 @@ fn test_chmod_permissions_too_large() { .ucmd() .args(&["10777", "file"]) .fails_with_code(1) - .stderr_is( - // spell-checker:disable-next-line - "chmod: mode is too large (10777 > 7777)\n", - ); + .usage_error("invalid mode: '10777'"); // test around the boundary of the acceptable octal mode scenario .ucmd() .args(&["10000", "file"]) .fails_with_code(1) - .stderr_is( - // spell-checker:disable-next-line - "chmod: mode is too large (10000 > 7777)\n", - ); + .usage_error("invalid mode: '10000'"); at.mkdir("dir"); scenario.ucmd().args(&["7777", "dir"]).succeeds(); } @@ -1820,7 +1834,7 @@ mod diagnostics { // The test harness pipes stderr, so the report must not appear. ucmd.args(&["g+rw?x", "probe"]) .fails_with_code(1) - .stderr_only("chmod: invalid operator (expected +, -, or =, but found ?)\n"); + .usage_error("invalid mode: 'g+rw?x'"); } #[cfg(unix)]