From ca07881243b866b38ac4aa49d7cca07aee599803 Mon Sep 17 00:00:00 2001 From: cerdelen Date: Wed, 26 Mar 2025 15:43:11 +0100 Subject: [PATCH 01/10] Parsing echo flags manually without clap as clap introduced various problematic interactions with hyphens --- src/uu/echo/src/echo.rs | 116 +++++++++++++++++++++++++------------ tests/by-util/test_echo.rs | 65 +++++++++++++++++++++ 2 files changed, 145 insertions(+), 36 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 7ce2fa9ad95..da9fee98c68 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. use clap::builder::ValueParser; -use clap::{Arg, ArgAction, ArgMatches, Command}; +use clap::{Arg, ArgAction, Command}; use std::env; use std::ffi::{OsStr, OsString}; use std::io::{self, StdoutLock, Write}; @@ -23,63 +23,107 @@ mod options { pub const DISABLE_BACKSLASH_ESCAPE: &str = "disable_backslash_escape"; } -fn is_echo_flag(arg: &OsString) -> bool { - matches!(arg.to_str(), Some("-e" | "-E" | "-n")) +struct EchoFlags { + pub n: bool, + pub e: bool, + pub is_hyphen: bool, } -// A workaround because clap interprets the first '--' as a marker that a value -// follows. In order to use '--' as a value, we have to inject an additional '--' -fn handle_double_hyphens(args: impl uucore::Args) -> impl uucore::Args { +fn is_echo_flag(arg: &OsString) -> Option{ + let bytes = arg.as_encoded_bytes(); + if bytes.first() == Some(&b'-') { + // this is a single hyphen which is pseudo flag (stops search for more flags but has no + // effect) + if arg.len() == 1 { + return Some(EchoFlags{ + n: true, + e: false, + is_hyphen: true + }); + } else { + let mut trailing_newline = true; + let mut escape = false; + + for c in &bytes[1..] { + match c { + b'e' => escape = true, + b'E' => escape = false, + b'n' => trailing_newline = false, + // if there is any char in an argument starting with '-' doesnt match e/E/n + // present means that this argument is not a flag + _ => return None, + } + } + return Some(EchoFlags{ + n: trailing_newline, + e: escape, + is_hyphen: false + }); + } + } + // argumetn doesnt start with '-' == no flag + None +} + +fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { let mut result = Vec::new(); - let mut is_first_argument = true; + let mut trailing_newline = true; + let mut escape = false; let mut args_iter = args.into_iter(); - if let Some(first_val) = args_iter.next() { - // the first argument ('echo') gets pushed before we start with the checks for flags/'--' - result.push(first_val); - // We need to skip any possible Flag arguments until we find the first argument to echo that - // is not a flag. If the first argument is double hyphen we inject an additional '--' - // otherwise we switch is_first_argument boolean to skip the checks for any further arguments - for arg in args_iter { - if is_first_argument && !is_echo_flag(&arg) { - is_first_argument = false; - if arg == "--" { - result.push(OsString::from("--")); - } + // We need to skip any possible Flag arguments until we find the first argument to echo that + // is not a flag. If the first argument is double hyphen we inject an additional '--' + // otherwise we switch is_first_argument boolean to skip the checks for any further arguments + for arg in &mut args_iter { + if let Some(echo_flags) = is_echo_flag(&arg) { + if echo_flags.is_hyphen { + // a single hyphen also breaks search for flags + result.push(arg); + break; } + trailing_newline = echo_flags.n; + escape = echo_flags.e; + } else { + // first found argument stops search for flags, from here everything is handled as a + // normal attribute result.push(arg); + break; } } - - result.into_iter() -} - -fn collect_args(matches: &ArgMatches) -> Vec { - matches - .get_many::(options::STRING) - .map_or_else(Vec::new, |values| values.cloned().collect()) + // push the remaining argumetns into result vector + for arg in args_iter { + result.push(arg); + } + (result, trailing_newline, escape) } #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let is_posixly_correct = env::var("POSIXLY_CORRECT").is_ok(); + let is_posixly_correct = if let Ok(posixly_correct) = env::var("POSIXLY_CORRECT") { + posixly_correct == "1" + } else { + false + }; + let args_iter = args.skip(1); let (args, trailing_newline, escaped) = if is_posixly_correct { - let mut args_iter = args.skip(1).peekable(); + let mut args_iter = args_iter.peekable(); if args_iter.peek() == Some(&OsString::from("-n")) { - let matches = uu_app().get_matches_from(handle_double_hyphens(args_iter)); - let args = collect_args(&matches); + // if POSIXLY_CORRECT is set and the first argument is the "-n" flag + // we filter flags normally but 'escaped' is activated nontheless + let (args, _, _) = filter_echo_flags(args_iter); (args, false, true) } else { - let args: Vec<_> = args_iter.collect(); + // if POSIXLY_CORRECT is set and the first argument is not the "-n" flag + // we just collect all arguments as every argument is considered an argument + let args: Vec = args_iter.collect(); (args, true, true) } + } else { - let matches = uu_app().get_matches_from(handle_double_hyphens(args.into_iter())); - let trailing_newline = !matches.get_flag(options::NO_NEWLINE); - let escaped = matches.get_flag(options::ENABLE_BACKSLASH_ESCAPE); - let args = collect_args(&matches); + // if POSIXLY_CORRECT is not set we filter the flags normally + let (args, trailing_newline, escaped) = filter_echo_flags(args_iter); (args, trailing_newline, escaped) }; diff --git a/tests/by-util/test_echo.rs b/tests/by-util/test_echo.rs index 9e2f686aff1..bf2e9b59709 100644 --- a/tests/by-util/test_echo.rs +++ b/tests/by-util/test_echo.rs @@ -273,6 +273,59 @@ fn test_double_hyphens_at_start() { .stdout_only("-- a b --\n"); } + +#[test] +fn test_double_hyphens_after_single_hyphen() { + new_ucmd!() + .arg("-") + .arg("--") + .succeeds() + .stdout_only("- --\n"); + + new_ucmd!() + .arg("-") + .arg("-n") + .arg("--") + .succeeds() + .stdout_only("- -n --\n"); + + new_ucmd!() + .arg("-n") + .arg("-") + .arg("--") + .succeeds() + .stdout_only("- --"); +} + +#[test] +fn test_flag_like_arguments_which_are_no_flags() { + new_ucmd!() + .arg("-efjkow") + .arg("--") + .succeeds() + .stdout_only("-efjkow --\n"); + + new_ucmd!() + .arg("--") + .arg("-efjkow") + .succeeds() + .stdout_only("-- -efjkow\n"); + + new_ucmd!() + .arg("-efjkow") + .arg("-n") + .arg("--") + .succeeds() + .stdout_only("-efjkow -n --\n"); + + new_ucmd!() + .arg("-n") + .arg("--") + .arg("-efjkow") + .succeeds() + .stdout_only("-- -efjkow"); +} + #[test] fn test_double_hyphens_after_flags() { new_ucmd!() @@ -289,6 +342,18 @@ fn test_double_hyphens_after_flags() { .succeeds() .stdout_only("-- foo\n"); + new_ucmd!() + .arg("-ne") + .arg("--") + .succeeds() + .stdout_only("--"); + + new_ucmd!() + .arg("-neE") + .arg("--") + .succeeds() + .stdout_only("--"); + new_ucmd!() .arg("-e") .arg("--") From 2ead244daa2a2e4f4dbbfd1da8bc96e645c244e0 Mon Sep 17 00:00:00 2001 From: cerdelen Date: Fri, 28 Mar 2025 16:07:48 +0100 Subject: [PATCH 02/10] fixed error where multiple flags would parse wrong --- src/uu/echo/src/echo.rs | 32 +++++++++++++++----------------- tests/by-util/test_echo.rs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index da9fee98c68..3b285006511 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -32,33 +32,29 @@ struct EchoFlags { fn is_echo_flag(arg: &OsString) -> Option{ let bytes = arg.as_encoded_bytes(); if bytes.first() == Some(&b'-') { + let mut flags = EchoFlags { + n: false, + e: false, + is_hyphen: false + }; // this is a single hyphen which is pseudo flag (stops search for more flags but has no // effect) if arg.len() == 1 { - return Some(EchoFlags{ - n: true, - e: false, - is_hyphen: true - }); + flags.is_hyphen = true; + return Some(flags); } else { - let mut trailing_newline = true; - let mut escape = false; - for c in &bytes[1..] { match c { - b'e' => escape = true, - b'E' => escape = false, - b'n' => trailing_newline = false, + b'e' => flags.e = true, + b'E' => flags.e = false, + b'n' => flags.n = true, // if there is any char in an argument starting with '-' doesnt match e/E/n // present means that this argument is not a flag _ => return None, } } - return Some(EchoFlags{ - n: trailing_newline, - e: escape, - is_hyphen: false - }); + + return Some(flags); } } // argumetn doesnt start with '-' == no flag @@ -81,7 +77,9 @@ fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { result.push(arg); break; } - trailing_newline = echo_flags.n; + if echo_flags.n { + trailing_newline = false; + } escape = echo_flags.e; } else { // first found argument stops search for flags, from here everything is handled as a diff --git a/tests/by-util/test_echo.rs b/tests/by-util/test_echo.rs index bf2e9b59709..cf6108d2453 100644 --- a/tests/by-util/test_echo.rs +++ b/tests/by-util/test_echo.rs @@ -326,6 +326,39 @@ fn test_flag_like_arguments_which_are_no_flags() { .stdout_only("-- -efjkow"); } + +#[test] +fn test_backshlash_n_last_char_in_last_argument() { + new_ucmd!() + .arg("-n") + .arg("-e") + .arg("--") + .arg("foo\n") + .succeeds() + .stdout_only("-- foo\n"); + + new_ucmd!() + .arg("-e") + .arg("--") + .arg("foo\\n") + .succeeds() + .stdout_only("-- foo\n\n"); + + new_ucmd!() + .arg("-n") + .arg("--") + .arg("foo\n") + .succeeds() + .stdout_only("-- foo\n"); + + new_ucmd!() + .arg("--") + .arg("foo\n") + .succeeds() + .stdout_only("-- foo\n\n"); +} + + #[test] fn test_double_hyphens_after_flags() { new_ucmd!() From 2f1a9d1e6c1ca40946948c4298c806ce7f3f1446 Mon Sep 17 00:00:00 2001 From: cerdelen Date: Fri, 28 Mar 2025 17:10:34 +0100 Subject: [PATCH 03/10] Spelling & formatting fixes --- src/uu/echo/src/echo.rs | 23 +++++++++++------------ tests/by-util/test_echo.rs | 7 ++----- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 3b285006511..31cf2873aad 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -29,14 +29,14 @@ struct EchoFlags { pub is_hyphen: bool, } -fn is_echo_flag(arg: &OsString) -> Option{ +fn is_echo_flag(arg: &OsString) -> Option { let bytes = arg.as_encoded_bytes(); if bytes.first() == Some(&b'-') { let mut flags = EchoFlags { - n: false, - e: false, - is_hyphen: false - }; + n: false, + e: false, + is_hyphen: false, + }; // this is a single hyphen which is pseudo flag (stops search for more flags but has no // effect) if arg.len() == 1 { @@ -48,7 +48,7 @@ fn is_echo_flag(arg: &OsString) -> Option{ b'e' => flags.e = true, b'E' => flags.e = false, b'n' => flags.n = true, - // if there is any char in an argument starting with '-' doesnt match e/E/n + // if there is any char in an argument starting with '-' doesn't match e/E/n // present means that this argument is not a flag _ => return None, } @@ -57,7 +57,7 @@ fn is_echo_flag(arg: &OsString) -> Option{ return Some(flags); } } - // argumetn doesnt start with '-' == no flag + // argument doesn't start with '-' == no flag None } @@ -88,7 +88,7 @@ fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { break; } } - // push the remaining argumetns into result vector + // push the remaining arguments into result vector for arg in args_iter { result.push(arg); } @@ -97,7 +97,7 @@ fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let is_posixly_correct = if let Ok(posixly_correct) = env::var("POSIXLY_CORRECT") { + let is_posixly_correct = if let Ok(posixly_correct) = env::var("POSIXLY_CORRECT") { posixly_correct == "1" } else { false @@ -109,16 +109,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { if args_iter.peek() == Some(&OsString::from("-n")) { // if POSIXLY_CORRECT is set and the first argument is the "-n" flag - // we filter flags normally but 'escaped' is activated nontheless + // we filter flags normally but 'escaped' is activated nonetheless let (args, _, _) = filter_echo_flags(args_iter); (args, false, true) } else { // if POSIXLY_CORRECT is set and the first argument is not the "-n" flag // we just collect all arguments as every argument is considered an argument - let args: Vec = args_iter.collect(); + let args: Vec = args_iter.collect(); (args, true, true) } - } else { // if POSIXLY_CORRECT is not set we filter the flags normally let (args, trailing_newline, escaped) = filter_echo_flags(args_iter); diff --git a/tests/by-util/test_echo.rs b/tests/by-util/test_echo.rs index cf6108d2453..dca43e69bcb 100644 --- a/tests/by-util/test_echo.rs +++ b/tests/by-util/test_echo.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (words) araba merci +// spell-checker:ignore (words) araba merci efjkow use crate::common::util::TestScenario; @@ -273,7 +273,6 @@ fn test_double_hyphens_at_start() { .stdout_only("-- a b --\n"); } - #[test] fn test_double_hyphens_after_single_hyphen() { new_ucmd!() @@ -326,9 +325,8 @@ fn test_flag_like_arguments_which_are_no_flags() { .stdout_only("-- -efjkow"); } - #[test] -fn test_backshlash_n_last_char_in_last_argument() { +fn test_backslash_n_last_char_in_last_argument() { new_ucmd!() .arg("-n") .arg("-e") @@ -358,7 +356,6 @@ fn test_backshlash_n_last_char_in_last_argument() { .stdout_only("-- foo\n\n"); } - #[test] fn test_double_hyphens_after_flags() { new_ucmd!() From c1c127edc70d02346364cd93f9241a5131654143 Mon Sep 17 00:00:00 2001 From: cerdelen Date: Sat, 29 Mar 2025 07:43:05 +0100 Subject: [PATCH 04/10] docu for EchoFlag struct --- src/uu/echo/src/echo.rs | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 31cf2873aad..3bcc01942ed 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -24,31 +24,37 @@ mod options { } struct EchoFlags { - pub n: bool, - pub e: bool, - pub is_hyphen: bool, + // n flag == true + // default = false + pub disable_newline: bool, + // e flag == true, E flag == false + // default = false + pub escape: bool, + // '-' argument + // default = false + pub is_single_hyphen: bool, } fn is_echo_flag(arg: &OsString) -> Option { let bytes = arg.as_encoded_bytes(); if bytes.first() == Some(&b'-') { let mut flags = EchoFlags { - n: false, - e: false, - is_hyphen: false, + disable_newline: false, + escape: false, + is_single_hyphen: false, }; // this is a single hyphen which is pseudo flag (stops search for more flags but has no // effect) if arg.len() == 1 { - flags.is_hyphen = true; + flags.is_single_hyphen = true; return Some(flags); } else { for c in &bytes[1..] { match c { - b'e' => flags.e = true, - b'E' => flags.e = false, - b'n' => flags.n = true, - // if there is any char in an argument starting with '-' doesn't match e/E/n + b'e' => flags.escape = true, + b'E' => flags.escape = false, + b'n' => flags.disable_newline = true, + // if there is any char in an argument starting with '-' that doesn't match e/E/n // present means that this argument is not a flag _ => return None, } @@ -72,15 +78,15 @@ fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { // otherwise we switch is_first_argument boolean to skip the checks for any further arguments for arg in &mut args_iter { if let Some(echo_flags) = is_echo_flag(&arg) { - if echo_flags.is_hyphen { + if echo_flags.is_single_hyphen { // a single hyphen also breaks search for flags result.push(arg); break; } - if echo_flags.n { + if echo_flags.disable_newline { trailing_newline = false; } - escape = echo_flags.e; + escape = echo_flags.escape; } else { // first found argument stops search for flags, from here everything is handled as a // normal attribute From ab95cf87ea65c4258c796d3a372e92e9d12f7eed Mon Sep 17 00:00:00 2001 From: cerdelen Date: Sat, 29 Mar 2025 11:14:21 +0100 Subject: [PATCH 05/10] more extensive comment/documentation --- src/uu/echo/src/echo.rs | 68 ++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 3bcc01942ed..1fe267d2ff4 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -23,18 +23,26 @@ mod options { pub const DISABLE_BACKSLASH_ESCAPE: &str = "disable_backslash_escape"; } +/// Holds the parsed flags for echo command: +/// -n (disable newline) +/// -e/-E (escape handling), +/// '-' (stop processing flags). struct EchoFlags { - // n flag == true - // default = false + /// -n flag: if true, don't output trailing newline + /// Default: false pub disable_newline: bool, - // e flag == true, E flag == false - // default = false + + /// -e enables escape interpretation, -E disables it + /// Default: false (escape interpretation disabled) pub escape: bool, - // '-' argument - // default = false + + /// Single hyphen '-' argument (stops flag processing) + /// Default: false pub is_single_hyphen: bool, } +/// Checks if an argument is a valid echo flag +/// Returns EchoFlags if valid, None otherwise fn is_echo_flag(arg: &OsString) -> Option { let bytes = arg.as_encoded_bytes(); if bytes.first() == Some(&b'-') { @@ -43,43 +51,47 @@ fn is_echo_flag(arg: &OsString) -> Option { escape: false, is_single_hyphen: false, }; - // this is a single hyphen which is pseudo flag (stops search for more flags but has no - // effect) + // Single hyphen case: "-" (stops flag processing, no other effect) if arg.len() == 1 { flags.is_single_hyphen = true; return Some(flags); - } else { - for c in &bytes[1..] { - match c { - b'e' => flags.escape = true, - b'E' => flags.escape = false, - b'n' => flags.disable_newline = true, - // if there is any char in an argument starting with '-' that doesn't match e/E/n - // present means that this argument is not a flag - _ => return None, - } - } + } - return Some(flags); + // Process characters after the '-' + for c in &bytes[1..] { + match c { + b'e' => flags.escape = true, + b'E' => flags.escape = false, + b'n' => flags.disable_newline = true, + // if there is any char in an argument starting with '-' that doesn't match e/E/n + // present means that this argument is not a flag + _ => return None, + } } + + return Some(flags); } - // argument doesn't start with '-' == no flag + + // argument doesn't start with '-' - no flag None } +/// Processes command line arguments, separating flags from normal arguments +/// Returns: +/// - Vector of non-flag arguments +/// - trailing_newline: whether to print a trailing newline +/// - escape: whether to process escape sequences fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { let mut result = Vec::new(); let mut trailing_newline = true; let mut escape = false; let mut args_iter = args.into_iter(); - // We need to skip any possible Flag arguments until we find the first argument to echo that - // is not a flag. If the first argument is double hyphen we inject an additional '--' - // otherwise we switch is_first_argument boolean to skip the checks for any further arguments + // Process arguments until first non-flag is found for arg in &mut args_iter { if let Some(echo_flags) = is_echo_flag(&arg) { + // Single hyphen stops flag processing if echo_flags.is_single_hyphen { - // a single hyphen also breaks search for flags result.push(arg); break; } @@ -88,13 +100,12 @@ fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { } escape = echo_flags.escape; } else { - // first found argument stops search for flags, from here everything is handled as a - // normal attribute + // First non-flag argument stops flag processing result.push(arg); break; } } - // push the remaining arguments into result vector + // Collect remaining arguments for arg in args_iter { result.push(arg); } @@ -103,6 +114,7 @@ fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { + // Check POSIX compatibility mode let is_posixly_correct = if let Ok(posixly_correct) = env::var("POSIXLY_CORRECT") { posixly_correct == "1" } else { From fcca66a023e8b408abf85fad410431d723e5004a Mon Sep 17 00:00:00 2001 From: cerdelen Date: Sat, 29 Mar 2025 12:41:33 +0100 Subject: [PATCH 06/10] revert POSIXLY_CORRECT check to only check if it is set --- src/uu/echo/src/echo.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 1fe267d2ff4..8b0477b15d6 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -115,11 +115,7 @@ fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Check POSIX compatibility mode - let is_posixly_correct = if let Ok(posixly_correct) = env::var("POSIXLY_CORRECT") { - posixly_correct == "1" - } else { - false - }; + let is_posixly_correct = env::var("POSIXLY_CORRECT").is_ok(); let args_iter = args.skip(1); let (args, trailing_newline, escaped) = if is_posixly_correct { From 5fc2c34352b516cf11120daf71901c400f6d0747 Mon Sep 17 00:00:00 2001 From: cerdelen Date: Mon, 31 Mar 2025 11:19:34 +0200 Subject: [PATCH 07/10] Fixed problem of overwriting flags. Added test for same issue --- src/uu/echo/src/echo.rs | 74 ++++++++++++++++---------------------- tests/by-util/test_echo.rs | 11 ++++++ 2 files changed, 41 insertions(+), 44 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 8b0477b15d6..7d740ed4999 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -23,57 +23,49 @@ mod options { pub const DISABLE_BACKSLASH_ESCAPE: &str = "disable_backslash_escape"; } -/// Holds the parsed flags for echo command: +/// Holds the options for echo command: /// -n (disable newline) /// -e/-E (escape handling), -/// '-' (stop processing flags). -struct EchoFlags { - /// -n flag: if true, don't output trailing newline - /// Default: false - pub disable_newline: bool, +struct EchoOptions { + /// -n flag option: if true, output a trailing newline (-n disables it) + /// Default: true + pub trailing_newline: bool, /// -e enables escape interpretation, -E disables it /// Default: false (escape interpretation disabled) pub escape: bool, - - /// Single hyphen '-' argument (stops flag processing) - /// Default: false - pub is_single_hyphen: bool, } /// Checks if an argument is a valid echo flag -/// Returns EchoFlags if valid, None otherwise -fn is_echo_flag(arg: &OsString) -> Option { +/// Returns true if valid echo flag found +fn is_echo_flag(arg: &OsString, echo_options: &mut EchoOptions) -> bool { let bytes = arg.as_encoded_bytes(); - if bytes.first() == Some(&b'-') { - let mut flags = EchoFlags { - disable_newline: false, - escape: false, - is_single_hyphen: false, - }; - // Single hyphen case: "-" (stops flag processing, no other effect) - if arg.len() == 1 { - flags.is_single_hyphen = true; - return Some(flags); - } + if bytes.first() == Some(&b'-') && arg!= "-" { + // we initialize our local variables to the "current" options so we dont override + // previous found flags + let mut escape = echo_options.escape; + let mut trailing_newline = echo_options.trailing_newline; // Process characters after the '-' for c in &bytes[1..] { match c { - b'e' => flags.escape = true, - b'E' => flags.escape = false, - b'n' => flags.disable_newline = true, + b'e' => escape = true, + b'E' => escape = false, + b'n' => trailing_newline = false, // if there is any char in an argument starting with '-' that doesn't match e/E/n // present means that this argument is not a flag - _ => return None, + _ => return false, } } - return Some(flags); + // we only override the options with flags being found once we parsed the whole argument + echo_options.escape = escape; + echo_options.trailing_newline = trailing_newline; + return true; } - // argument doesn't start with '-' - no flag - None + // argument doesn't start with '-' or is "-" => no flag + false } /// Processes command line arguments, separating flags from normal arguments @@ -83,23 +75,17 @@ fn is_echo_flag(arg: &OsString) -> Option { /// - escape: whether to process escape sequences fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { let mut result = Vec::new(); - let mut trailing_newline = true; - let mut escape = false; + let mut echo_options = EchoOptions{ + trailing_newline: true, + escape: false, + }; let mut args_iter = args.into_iter(); // Process arguments until first non-flag is found for arg in &mut args_iter { - if let Some(echo_flags) = is_echo_flag(&arg) { - // Single hyphen stops flag processing - if echo_flags.is_single_hyphen { - result.push(arg); - break; - } - if echo_flags.disable_newline { - trailing_newline = false; - } - escape = echo_flags.escape; - } else { + // we parse flags and store options found in "echo_option". First is_echo_flag + // call to return false will break the loop and we will collect the remaining argumetns + if !is_echo_flag(&arg, &mut echo_options) { // First non-flag argument stops flag processing result.push(arg); break; @@ -109,7 +95,7 @@ fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { for arg in args_iter { result.push(arg); } - (result, trailing_newline, escape) + (result, echo_options.trailing_newline, echo_options.escape) } #[uucore::main] diff --git a/tests/by-util/test_echo.rs b/tests/by-util/test_echo.rs index 6ef13c1579f..868f45e7e7d 100644 --- a/tests/by-util/test_echo.rs +++ b/tests/by-util/test_echo.rs @@ -126,6 +126,17 @@ fn test_escape_override() { .args(&["-E", "-e", "\\na"]) .succeeds() .stdout_only("\na\n"); + + + new_ucmd!() + .args(&["-E", "-e", "-n", "\\na"]) + .succeeds() + .stdout_only("\na"); + + new_ucmd!() + .args(&["-e", "-E", "-n", "\\na"]) + .succeeds() + .stdout_only("\\na"); } #[test] From bb165133da69283e7862b0158c4a7198eb84d659 Mon Sep 17 00:00:00 2001 From: cerdelen Date: Mon, 31 Mar 2025 11:21:12 +0200 Subject: [PATCH 08/10] cargo fmt --- src/uu/echo/src/echo.rs | 4 ++-- tests/by-util/test_echo.rs | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 7d740ed4999..a9272d7a573 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -40,7 +40,7 @@ struct EchoOptions { /// Returns true if valid echo flag found fn is_echo_flag(arg: &OsString, echo_options: &mut EchoOptions) -> bool { let bytes = arg.as_encoded_bytes(); - if bytes.first() == Some(&b'-') && arg!= "-" { + if bytes.first() == Some(&b'-') && arg != "-" { // we initialize our local variables to the "current" options so we dont override // previous found flags let mut escape = echo_options.escape; @@ -75,7 +75,7 @@ fn is_echo_flag(arg: &OsString, echo_options: &mut EchoOptions) -> bool { /// - escape: whether to process escape sequences fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { let mut result = Vec::new(); - let mut echo_options = EchoOptions{ + let mut echo_options = EchoOptions { trailing_newline: true, escape: false, }; diff --git a/tests/by-util/test_echo.rs b/tests/by-util/test_echo.rs index 868f45e7e7d..17545a7887b 100644 --- a/tests/by-util/test_echo.rs +++ b/tests/by-util/test_echo.rs @@ -127,7 +127,6 @@ fn test_escape_override() { .succeeds() .stdout_only("\na\n"); - new_ucmd!() .args(&["-E", "-e", "-n", "\\na"]) .succeeds() From 17c6c21b37a9e03d302dc16723b59dff555eb176 Mon Sep 17 00:00:00 2001 From: cerdelen Date: Mon, 31 Mar 2025 11:26:50 +0200 Subject: [PATCH 09/10] cspell --- src/uu/echo/src/echo.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index a9272d7a573..38371bfbea5 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -41,7 +41,7 @@ struct EchoOptions { fn is_echo_flag(arg: &OsString, echo_options: &mut EchoOptions) -> bool { let bytes = arg.as_encoded_bytes(); if bytes.first() == Some(&b'-') && arg != "-" { - // we initialize our local variables to the "current" options so we dont override + // we initialize our local variables to the "current" options so we don't override // previous found flags let mut escape = echo_options.escape; let mut trailing_newline = echo_options.trailing_newline; @@ -84,7 +84,7 @@ fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { // Process arguments until first non-flag is found for arg in &mut args_iter { // we parse flags and store options found in "echo_option". First is_echo_flag - // call to return false will break the loop and we will collect the remaining argumetns + // call to return false will break the loop and we will collect the remaining arguments if !is_echo_flag(&arg, &mut echo_options) { // First non-flag argument stops flag processing result.push(arg); From e8c3e42a5bde68540ecc2fb486b4cb56e5ba9e4e Mon Sep 17 00:00:00 2001 From: cerdelen <95369756+cerdelen@users.noreply.github.com> Date: Mon, 31 Mar 2025 12:34:27 +0200 Subject: [PATCH 10/10] Update src/uu/echo/src/echo.rs Enabling POSIXLY_CORRECT flag if value is not UTF-8 Co-authored-by: Jan Verbeek --- src/uu/echo/src/echo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 38371bfbea5..1a755c6965f 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -101,7 +101,7 @@ fn filter_echo_flags(args: impl uucore::Args) -> (Vec, bool, bool) { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Check POSIX compatibility mode - let is_posixly_correct = env::var("POSIXLY_CORRECT").is_ok(); + let is_posixly_correct = env::var_os("POSIXLY_CORRECT").is_some(); let args_iter = args.skip(1); let (args, trailing_newline, escaped) = if is_posixly_correct {