From 836351dd7a8920534b1ac64ee0c2ff46a391ba18 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 28 Dec 2024 23:41:14 +0100 Subject: [PATCH 1/4] ch*: refactor duplicate declarations --- src/uu/chgrp/src/chgrp.rs | 30 ++++++++------------------ src/uu/chmod/src/chmod.rs | 18 +++++++++++----- src/uu/chown/src/chown.rs | 32 ++++++++-------------------- src/uucore/src/lib/features/perms.rs | 21 ++++++++++++++++++ 4 files changed, 52 insertions(+), 49 deletions(-) diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs index fba2cef16..7f68f7b13 100644 --- a/src/uu/chgrp/src/chgrp.rs +++ b/src/uu/chgrp/src/chgrp.rs @@ -63,7 +63,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } pub fn uu_app() -> Command { - Command::new(uucore::util_name()) + let mut cmd = Command::new(uucore::util_name()) .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) @@ -140,24 +140,12 @@ pub fn uu_app() -> Command { .long(options::RECURSIVE) .help("operate on files and directories recursively") .action(ArgAction::SetTrue), - ) - .arg( - Arg::new(options::traverse::TRAVERSE) - .short(options::traverse::TRAVERSE.chars().next().unwrap()) - .help("if a command line argument is a symbolic link to a directory, traverse it") - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new(options::traverse::NO_TRAVERSE) - .short(options::traverse::NO_TRAVERSE.chars().next().unwrap()) - .help("do not traverse any symbolic links (default)") - .overrides_with_all([options::traverse::TRAVERSE, options::traverse::EVERY]) - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new(options::traverse::EVERY) - .short(options::traverse::EVERY.chars().next().unwrap()) - .help("traverse every symbolic link to a directory encountered") - .action(ArgAction::SetTrue), - ) + ); + + // Add traverse-related arguments + for arg in uucore::perms::traverse_args() { + cmd = cmd.arg(arg); + } + + cmd } diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index d13257437..07c04a98d 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -151,7 +151,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } pub fn uu_app() -> Command { - Command::new(uucore::util_name()) + let mut cmd = Command::new(uucore::util_name()) .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) @@ -206,16 +206,24 @@ pub fn uu_app() -> Command { .help("use RFILE's mode instead of MODE values"), ) .arg( - Arg::new(options::MODE).required_unless_present(options::REFERENCE), // It would be nice if clap could parse with delimiter, e.g. "g-x,u+x", - // however .multiple_occurrences(true) cannot be used here because FILE already needs that. - // Only one positional argument with .multiple_occurrences(true) set is allowed per command + Arg::new(options::MODE).required_unless_present(options::REFERENCE), + // It would be nice if clap could parse with delimiter, e.g. "g-x,u+x", + // however .multiple_occurrences(true) cannot be used here because FILE already needs that. + // Only one positional argument with .multiple_occurrences(true) set is allowed per command ) .arg( Arg::new(options::FILE) .required_unless_present(options::MODE) .action(ArgAction::Append) .value_hint(clap::ValueHint::AnyPath), - ) + ); + + // Add traverse-related arguments + for arg in uucore::perms::traverse_args() { + cmd = cmd.arg(arg); + } + + cmd } struct Chmoder { diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 0e9b8b242..61fe24eab 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -77,7 +77,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } pub fn uu_app() -> Command { - Command::new(uucore::util_name()) + let mut cmd = Command::new(uucore::util_name()) .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) @@ -165,34 +165,20 @@ pub fn uu_app() -> Command { .long(options::verbosity::SILENT) .action(ArgAction::SetTrue), ) - .arg( - Arg::new(options::traverse::TRAVERSE) - .short(options::traverse::TRAVERSE.chars().next().unwrap()) - .help("if a command line argument is a symbolic link to a directory, traverse it") - .overrides_with_all([options::traverse::EVERY, options::traverse::NO_TRAVERSE]) - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new(options::traverse::EVERY) - .short(options::traverse::EVERY.chars().next().unwrap()) - .help("traverse every symbolic link to a directory encountered") - .overrides_with_all([options::traverse::TRAVERSE, options::traverse::NO_TRAVERSE]) - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new(options::traverse::NO_TRAVERSE) - .short(options::traverse::NO_TRAVERSE.chars().next().unwrap()) - .help("do not traverse any symbolic links (default)") - .overrides_with_all([options::traverse::TRAVERSE, options::traverse::EVERY]) - .action(ArgAction::SetTrue), - ) .arg( Arg::new(options::verbosity::VERBOSE) .long(options::verbosity::VERBOSE) .short('v') .help("output a diagnostic for every file processed") .action(ArgAction::SetTrue), - ) + ); + + // Add traverse-related arguments + for arg in uucore::perms::traverse_args() { + cmd = cmd.arg(arg); + } + + cmd } /// Parses the user string to extract the UID. diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 3623e9e61..9ee64e212 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -13,6 +13,7 @@ pub use crate::features::entries; use crate::show_error; use clap::{Arg, ArgMatches, Command}; use libc::{gid_t, uid_t}; +use options::traverse; use walkdir::WalkDir; use std::io::Error as IOError; @@ -634,6 +635,26 @@ pub fn chown_base( executor.exec() } +pub fn traverse_args() -> Vec { + vec![ + Arg::new(traverse::TRAVERSE) + .short(traverse::TRAVERSE.chars().next().unwrap()) + .help("if a command line argument is a symbolic link to a directory, traverse it") + .overrides_with_all([traverse::EVERY, traverse::NO_TRAVERSE]) + .action(clap::ArgAction::SetTrue), + Arg::new(traverse::EVERY) + .short(traverse::EVERY.chars().next().unwrap()) + .help("traverse every symbolic link to a directory encountered") + .overrides_with_all([traverse::TRAVERSE, traverse::NO_TRAVERSE]) + .action(clap::ArgAction::SetTrue), + Arg::new(traverse::NO_TRAVERSE) + .short(traverse::NO_TRAVERSE.chars().next().unwrap()) + .help("do not traverse any symbolic links (default)") + .overrides_with_all([traverse::TRAVERSE, traverse::EVERY]) + .action(clap::ArgAction::SetTrue), + ] +} + #[cfg(test)] mod tests { // Note this useful idiom: importing names from outer (for mod tests) scope. From 6de5aa7ac7b33fb32d530908a7eb16591341ae17 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 28 Dec 2024 23:41:58 +0100 Subject: [PATCH 2/4] ch*: improve the presentation --- src/uucore/src/lib/features/perms.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 9ee64e212..9b59f7259 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -34,6 +34,7 @@ pub enum VerbosityLevel { Verbose, Normal, } + #[derive(PartialEq, Eq, Clone, Debug)] pub struct Verbosity { pub groups_only: bool, From e45c56b926cdd8d5c346513fd06c3569085588ef Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 29 Dec 2024 00:17:22 +0100 Subject: [PATCH 3/4] ch*: also remove duplications for deref & no deref --- src/uu/chgrp/src/chgrp.rs | 20 +++----------------- src/uu/chmod/src/chmod.rs | 12 ++++++++++-- src/uu/chown/src/chown.rs | 23 ++--------------------- src/uucore/src/lib/features/perms.rs | 17 ++++++++++++++++- 4 files changed, 31 insertions(+), 41 deletions(-) diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs index 7f68f7b13..16f67144a 100644 --- a/src/uu/chgrp/src/chgrp.rs +++ b/src/uu/chgrp/src/chgrp.rs @@ -73,7 +73,7 @@ pub fn uu_app() -> Command { Arg::new(options::HELP) .long(options::HELP) .help("Print help information.") - .action(ArgAction::Help) + .action(ArgAction::Help), ) .arg( Arg::new(options::verbosity::CHANGES) @@ -101,20 +101,6 @@ pub fn uu_app() -> Command { .help("output a diagnostic for every file processed") .action(ArgAction::SetTrue), ) - .arg( - Arg::new(options::dereference::DEREFERENCE) - .long(options::dereference::DEREFERENCE) - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new(options::dereference::NO_DEREFERENCE) - .short('h') - .long(options::dereference::NO_DEREFERENCE) - .help( - "affect symbolic links instead of any referenced file (useful only on systems that can change the ownership of a symlink)", - ) - .action(ArgAction::SetTrue), - ) .arg( Arg::new(options::preserve_root::PRESERVE) .long(options::preserve_root::PRESERVE) @@ -142,8 +128,8 @@ pub fn uu_app() -> Command { .action(ArgAction::SetTrue), ); - // Add traverse-related arguments - for arg in uucore::perms::traverse_args() { + // Add common arguments with chgrp, chown & chmod + for arg in uucore::perms::common_args() { cmd = cmd.arg(arg); } diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 07c04a98d..f62f66503 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -23,6 +23,7 @@ const USAGE: &str = help_usage!("chmod.md"); const LONG_USAGE: &str = help_section!("after help", "chmod.md"); mod options { + pub const HELP: &str = "help"; pub const CHANGES: &str = "changes"; pub const QUIET: &str = "quiet"; // visible_alias("silent") pub const VERBOSE: &str = "verbose"; @@ -158,6 +159,13 @@ pub fn uu_app() -> Command { .args_override_self(true) .infer_long_args(true) .no_binary_name(true) + .disable_help_flag(true) + .arg( + Arg::new(options::HELP) + .long(options::HELP) + .help("Print help information.") + .action(ArgAction::Help), + ) .arg( Arg::new(options::CHANGES) .long(options::CHANGES) @@ -218,8 +226,8 @@ pub fn uu_app() -> Command { .value_hint(clap::ValueHint::AnyPath), ); - // Add traverse-related arguments - for arg in uucore::perms::traverse_args() { + // Add common arguments with chgrp, chown & chmod + for arg in uucore::perms::common_args() { cmd = cmd.arg(arg); } diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 61fe24eab..d8cb14fa3 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -96,25 +96,6 @@ pub fn uu_app() -> Command { .help("like verbose but report only when a change is made") .action(ArgAction::SetTrue), ) - .arg( - Arg::new(options::dereference::DEREFERENCE) - .long(options::dereference::DEREFERENCE) - .help( - "affect the referent of each symbolic link (this is the default), \ - rather than the symbolic link itself", - ) - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new(options::dereference::NO_DEREFERENCE) - .short('h') - .long(options::dereference::NO_DEREFERENCE) - .help( - "affect symbolic links instead of any referenced file \ - (useful only on systems that can change the ownership of a symlink)", - ) - .action(ArgAction::SetTrue), - ) .arg( Arg::new(options::FROM) .long(options::FROM) @@ -173,8 +154,8 @@ pub fn uu_app() -> Command { .action(ArgAction::SetTrue), ); - // Add traverse-related arguments - for arg in uucore::perms::traverse_args() { + // Add common arguments with chgrp, chown & chmod + for arg in uucore::perms::common_args() { cmd = cmd.arg(arg); } diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 9b59f7259..73b84be72 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -636,7 +636,7 @@ pub fn chown_base( executor.exec() } -pub fn traverse_args() -> Vec { +pub fn common_args() -> Vec { vec![ Arg::new(traverse::TRAVERSE) .short(traverse::TRAVERSE.chars().next().unwrap()) @@ -653,6 +653,21 @@ pub fn traverse_args() -> Vec { .help("do not traverse any symbolic links (default)") .overrides_with_all([traverse::TRAVERSE, traverse::EVERY]) .action(clap::ArgAction::SetTrue), + Arg::new(options::dereference::DEREFERENCE) + .long(options::dereference::DEREFERENCE) + .help( + "affect the referent of each symbolic link (this is the default), \ + rather than the symbolic link itself", + ) + .action(clap::ArgAction::SetTrue), + Arg::new(options::dereference::NO_DEREFERENCE) + .short('h') + .long(options::dereference::NO_DEREFERENCE) + .help( + "affect symbolic links instead of any referenced file \ + (useful only on systems that can change the ownership of a symlink)", + ) + .action(clap::ArgAction::SetTrue), ] } From 29694212dd9423ce3a221bc32cdf9636d941bde8 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 29 Dec 2024 16:36:14 +0100 Subject: [PATCH 4/4] refactor: simplify the code by using the clap function instead --- src/uu/chgrp/src/chgrp.rs | 13 ++++--------- src/uu/chmod/src/chmod.rs | 13 ++++--------- src/uu/chown/src/chown.rs | 13 ++++--------- 3 files changed, 12 insertions(+), 27 deletions(-) diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs index 16f67144a..fe5aee872 100644 --- a/src/uu/chgrp/src/chgrp.rs +++ b/src/uu/chgrp/src/chgrp.rs @@ -63,7 +63,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } pub fn uu_app() -> Command { - let mut cmd = Command::new(uucore::util_name()) + Command::new(uucore::util_name()) .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) @@ -126,12 +126,7 @@ pub fn uu_app() -> Command { .long(options::RECURSIVE) .help("operate on files and directories recursively") .action(ArgAction::SetTrue), - ); - - // Add common arguments with chgrp, chown & chmod - for arg in uucore::perms::common_args() { - cmd = cmd.arg(arg); - } - - cmd + ) + // Add common arguments with chgrp, chown & chmod + .args(uucore::perms::common_args()) } diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index f62f66503..c05288e21 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -152,7 +152,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } pub fn uu_app() -> Command { - let mut cmd = Command::new(uucore::util_name()) + Command::new(uucore::util_name()) .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) @@ -224,14 +224,9 @@ pub fn uu_app() -> Command { .required_unless_present(options::MODE) .action(ArgAction::Append) .value_hint(clap::ValueHint::AnyPath), - ); - - // Add common arguments with chgrp, chown & chmod - for arg in uucore::perms::common_args() { - cmd = cmd.arg(arg); - } - - cmd + ) + // Add common arguments with chgrp, chown & chmod + .args(uucore::perms::common_args()) } struct Chmoder { diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index d8cb14fa3..20bc87c34 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -77,7 +77,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } pub fn uu_app() -> Command { - let mut cmd = Command::new(uucore::util_name()) + Command::new(uucore::util_name()) .version(crate_version!()) .about(ABOUT) .override_usage(format_usage(USAGE)) @@ -152,14 +152,9 @@ pub fn uu_app() -> Command { .short('v') .help("output a diagnostic for every file processed") .action(ArgAction::SetTrue), - ); - - // Add common arguments with chgrp, chown & chmod - for arg in uucore::perms::common_args() { - cmd = cmd.arg(arg); - } - - cmd + ) + // Add common arguments with chgrp, chown & chmod + .args(uucore::perms::common_args()) } /// Parses the user string to extract the UID.