From e30f191579c4fe1b5c64614cf9712feb9888dc66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dorian=20P=C3=A9ron?= Date: Wed, 3 Jan 2024 18:37:51 +0100 Subject: [PATCH 1/4] ls: Handle the use of QUOTING_STYLE variable --- src/uu/ls/src/ls.rs | 111 +++++++++++++++++++++++++++++--------------- 1 file changed, 73 insertions(+), 38 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 952083d2a..d56929be8 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -621,7 +621,52 @@ fn extract_hyperlink(options: &clap::ArgMatches) -> bool { } } +/// Match the argument given to --quoting-style or the QUOTING_STYLE env variable. +/// +/// # Arguments +/// +/// * `style`: the actual argument string +/// * `show_control` - A boolean value representing whether or not to show control characters. +/// +/// # Returns +/// +/// * An option with None if the style string is invalid, or a `QuotingStyle` wrapped in `Some`. +fn match_quoting_style_name(style: &str, show_control: bool) -> Option { + match style { + "literal" => Some(QuotingStyle::Literal { show_control }), + "shell" => Some(QuotingStyle::Shell { + escape: false, + always_quote: false, + show_control, + }), + "shell-always" => Some(QuotingStyle::Shell { + escape: false, + always_quote: true, + show_control, + }), + "shell-escape" => Some(QuotingStyle::Shell { + escape: true, + always_quote: false, + show_control, + }), + "shell-escape-always" => Some(QuotingStyle::Shell { + escape: true, + always_quote: true, + show_control, + }), + "c" => Some(QuotingStyle::C { + quotes: quoting_style::Quotes::Double, + }), + "escape" => Some(QuotingStyle::C { + quotes: quoting_style::Quotes::None, + }), + _ => None, + } +} + /// Extracts the quoting style to use based on the options provided. +/// If no options are given, it looks if a default quoting style is provided +/// through the QUOTING_STYLE environment variable. /// /// # Arguments /// @@ -632,38 +677,12 @@ fn extract_hyperlink(options: &clap::ArgMatches) -> bool { /// /// A QuotingStyle variant representing the quoting style to use. fn extract_quoting_style(options: &clap::ArgMatches, show_control: bool) -> QuotingStyle { - let opt_quoting_style = options.get_one::(options::QUOTING_STYLE).cloned(); + let opt_quoting_style = options.get_one::(options::QUOTING_STYLE); if let Some(style) = opt_quoting_style { - match style.as_str() { - "literal" => QuotingStyle::Literal { show_control }, - "shell" => QuotingStyle::Shell { - escape: false, - always_quote: false, - show_control, - }, - "shell-always" => QuotingStyle::Shell { - escape: false, - always_quote: true, - show_control, - }, - "shell-escape" => QuotingStyle::Shell { - escape: true, - always_quote: false, - show_control, - }, - "shell-escape-always" => QuotingStyle::Shell { - escape: true, - always_quote: true, - show_control, - }, - "c" => QuotingStyle::C { - quotes: quoting_style::Quotes::Double, - }, - "escape" => QuotingStyle::C { - quotes: quoting_style::Quotes::None, - }, - _ => unreachable!("Should have been caught by Clap"), + match match_quoting_style_name(style, show_control) { + Some(qs) => qs, + None => unreachable!("Should have been caught by Clap"), } } else if options.get_flag(options::quoting::LITERAL) { QuotingStyle::Literal { show_control } @@ -675,16 +694,32 @@ fn extract_quoting_style(options: &clap::ArgMatches, show_control: bool) -> Quot QuotingStyle::C { quotes: quoting_style::Quotes::Double, } - } else if options.get_flag(options::DIRED) || !std::io::stdout().is_terminal() { - // By default, `ls` uses Literal quoting when - // writing to a non-terminal file descriptor + } else if options.get_flag(options::DIRED) { QuotingStyle::Literal { show_control } } else { - // TODO: use environment variable if available - QuotingStyle::Shell { - escape: true, - always_quote: false, - show_control, + // If set, the QUOTING_STYLE environment variable specifies a default style. + if let Ok(style) = std::env::var("QUOTING_STYLE") { + match match_quoting_style_name(style.as_str(), show_control) { + Some(qs) => return qs, + None => eprintln!( + "{}: Ignoring invalid value of environment variable QUOTING_STYLE: '{}'", + std::env::args().next().unwrap_or("ls".to_string()), + style + ), + } + } + + // By default, `ls` uses Literal quoting when + // writing to a non-terminal file descriptor + if !std::io::stdout().is_terminal() { + QuotingStyle::Literal { show_control } + } else { + // TODO: use environment variable if available + QuotingStyle::Shell { + escape: true, + always_quote: false, + show_control, + } } } } From 6760d63539cd2bb8ac1e5ec345baea84a1c902a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dorian=20P=C3=A9ron?= Date: Thu, 4 Jan 2024 16:51:30 +0100 Subject: [PATCH 2/4] ls: Fix clippy warning --- src/uu/ls/src/ls.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index d56929be8..0e9b25722 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -709,17 +709,16 @@ fn extract_quoting_style(options: &clap::ArgMatches, show_control: bool) -> Quot } } - // By default, `ls` uses Literal quoting when - // writing to a non-terminal file descriptor - if !std::io::stdout().is_terminal() { - QuotingStyle::Literal { show_control } - } else { - // TODO: use environment variable if available + // By default, `ls` uses Shell escape quoting style when writing to a terminal file + // descriptor and Literal otherwise. + if std::io::stdout().is_terminal() { QuotingStyle::Shell { escape: true, always_quote: false, show_control, } + } else { + QuotingStyle::Literal { show_control } } } } From c58575edaaa208622077f4c30cf80645e06e1c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dorian=20P=C3=A9ron?= Date: Fri, 5 Jan 2024 02:07:09 +0100 Subject: [PATCH 3/4] tests/ls: Add tests to ensure env var is used as a last resort --- tests/by-util/test_ls.rs | 65 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 72a303ef3..476704660 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2622,6 +2622,71 @@ fn test_ls_quoting_style() { } } +#[test] +fn test_ls_quoting_style_env_var_default() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch(at.plus_as_string("foo-1")); + at.touch(at.plus_as_string("bar-2")); + + // If no quoting style argument is provided, the QUOTING_STYLE environment variable + // shall be used. + + let correct_c = "\"bar-2\"\n\"foo-1\""; + scene + .ucmd() + .env("QUOTING_STYLE", "c") + .succeeds() + .stdout_only(format!("{correct_c}\n")); +} + +#[test] +fn test_ls_quoting_style_arg_overrides_env_var() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch(at.plus_as_string("foo-1")); + at.touch(at.plus_as_string("bar-2")); + + // The quoting style given by the env variable should be + // overriden by any escape style provided by argument. + for (arg, correct) in [ + ("--quoting-style=literal", "foo-1"), + ("-N", "foo-1"), + ("--quoting-style=escape", "foo-1"), + ("-b", "foo-1"), + ("--quoting-style=shell-escape", "foo-1"), + ("--quoting-style=shell-escape-always", "'foo-1'"), + ("--quoting-style=shell", "foo-1"), + ("--quoting-style=shell-always", "'foo-1'"), + ] { + scene + .ucmd() + .env("QUOTING_STYLE", "c") + .arg("--hide-control-chars") + .arg(arg) + .arg("foo-1") + .succeeds() + .stdout_only(format!("{correct}\n")); + } + + // Another loop to check for the C quoting style that is used as a default above. + for (arg, correct) in [ + ("--quoting-style=c", "\"foo-1\""), + ("-Q", "\"foo-1\""), + ("--quote-name", "\"foo-1\""), + ] { + scene + .ucmd() + .env("QUOTING_STYLE", "literal") + .arg("--hide-control-chars") + .arg(arg) + .arg("foo-1") + .succeeds() + .stdout_only(format!("{correct}\n")); + } + +} + #[test] fn test_ls_quoting_and_color() { let scene = TestScenario::new(util_name!()); From 4372908e8487416b93836ba5d926b5cc441cd11a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dorian=20P=C3=A9ron?= Date: Fri, 5 Jan 2024 13:51:28 +0100 Subject: [PATCH 4/4] fix: cargo fmt + fix spelling mistake --- tests/by-util/test_ls.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 476704660..0162b0170 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2648,7 +2648,7 @@ fn test_ls_quoting_style_arg_overrides_env_var() { at.touch(at.plus_as_string("bar-2")); // The quoting style given by the env variable should be - // overriden by any escape style provided by argument. + // overridden by any escape style provided by argument. for (arg, correct) in [ ("--quoting-style=literal", "foo-1"), ("-N", "foo-1"), @@ -2684,7 +2684,6 @@ fn test_ls_quoting_style_arg_overrides_env_var() { .succeeds() .stdout_only(format!("{correct}\n")); } - } #[test]