Skip to content

Commit d84989d

Browse files
cerdelenblyxxyz
authored andcommitted
Remove clap for echo (uutils#7603)
* Parsing echo flags manually without clap as clap introduced various problematic interactions with hyphens * fixed error where multiple flags would parse wrong * Spelling & formatting fixes * docu for EchoFlag struct * more extensive comment/documentation * revert POSIXLY_CORRECT check to only check if it is set * Fixed problem of overwriting flags. Added test for same issue * cargo fmt * cspell * Update src/uu/echo/src/echo.rs Enabling POSIXLY_CORRECT flag if value is not UTF-8 Co-authored-by: Jan Verbeek <[email protected]> --------- Co-authored-by: Jan Verbeek <[email protected]>
1 parent 0a5c32d commit d84989d

File tree

2 files changed

+184
-38
lines changed

2 files changed

+184
-38
lines changed

src/uu/echo/src/echo.rs

Lines changed: 78 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// file that was distributed with this source code.
55

66
use clap::builder::ValueParser;
7-
use clap::{Arg, ArgAction, ArgMatches, Command};
7+
use clap::{Arg, ArgAction, Command};
88
use std::env;
99
use std::ffi::{OsStr, OsString};
1010
use std::io::{self, StdoutLock, Write};
@@ -23,63 +23,104 @@ mod options {
2323
pub const DISABLE_BACKSLASH_ESCAPE: &str = "disable_backslash_escape";
2424
}
2525

26-
fn is_echo_flag(arg: &OsString) -> bool {
27-
matches!(arg.to_str(), Some("-e" | "-E" | "-n"))
26+
/// Holds the options for echo command:
27+
/// -n (disable newline)
28+
/// -e/-E (escape handling),
29+
struct EchoOptions {
30+
/// -n flag option: if true, output a trailing newline (-n disables it)
31+
/// Default: true
32+
pub trailing_newline: bool,
33+
34+
/// -e enables escape interpretation, -E disables it
35+
/// Default: false (escape interpretation disabled)
36+
pub escape: bool,
2837
}
2938

30-
// A workaround because clap interprets the first '--' as a marker that a value
31-
// follows. In order to use '--' as a value, we have to inject an additional '--'
32-
fn handle_double_hyphens(args: impl uucore::Args) -> impl uucore::Args {
33-
let mut result = Vec::new();
34-
let mut is_first_argument = true;
35-
let mut args_iter = args.into_iter();
36-
37-
if let Some(first_val) = args_iter.next() {
38-
// the first argument ('echo') gets pushed before we start with the checks for flags/'--'
39-
result.push(first_val);
40-
// We need to skip any possible Flag arguments until we find the first argument to echo that
41-
// is not a flag. If the first argument is double hyphen we inject an additional '--'
42-
// otherwise we switch is_first_argument boolean to skip the checks for any further arguments
43-
for arg in args_iter {
44-
if is_first_argument && !is_echo_flag(&arg) {
45-
is_first_argument = false;
46-
if arg == "--" {
47-
result.push(OsString::from("--"));
48-
}
39+
/// Checks if an argument is a valid echo flag
40+
/// Returns true if valid echo flag found
41+
fn is_echo_flag(arg: &OsString, echo_options: &mut EchoOptions) -> bool {
42+
let bytes = arg.as_encoded_bytes();
43+
if bytes.first() == Some(&b'-') && arg != "-" {
44+
// we initialize our local variables to the "current" options so we don't override
45+
// previous found flags
46+
let mut escape = echo_options.escape;
47+
let mut trailing_newline = echo_options.trailing_newline;
48+
49+
// Process characters after the '-'
50+
for c in &bytes[1..] {
51+
match c {
52+
b'e' => escape = true,
53+
b'E' => escape = false,
54+
b'n' => trailing_newline = false,
55+
// if there is any char in an argument starting with '-' that doesn't match e/E/n
56+
// present means that this argument is not a flag
57+
_ => return false,
4958
}
50-
result.push(arg);
5159
}
60+
61+
// we only override the options with flags being found once we parsed the whole argument
62+
echo_options.escape = escape;
63+
echo_options.trailing_newline = trailing_newline;
64+
return true;
5265
}
5366

54-
result.into_iter()
67+
// argument doesn't start with '-' or is "-" => no flag
68+
false
5569
}
5670

57-
fn collect_args(matches: &ArgMatches) -> Vec<OsString> {
58-
matches
59-
.get_many::<OsString>(options::STRING)
60-
.map_or_else(Vec::new, |values| values.cloned().collect())
71+
/// Processes command line arguments, separating flags from normal arguments
72+
/// Returns:
73+
/// - Vector of non-flag arguments
74+
/// - trailing_newline: whether to print a trailing newline
75+
/// - escape: whether to process escape sequences
76+
fn filter_echo_flags(args: impl uucore::Args) -> (Vec<OsString>, bool, bool) {
77+
let mut result = Vec::new();
78+
let mut echo_options = EchoOptions {
79+
trailing_newline: true,
80+
escape: false,
81+
};
82+
let mut args_iter = args.into_iter();
83+
84+
// Process arguments until first non-flag is found
85+
for arg in &mut args_iter {
86+
// we parse flags and store options found in "echo_option". First is_echo_flag
87+
// call to return false will break the loop and we will collect the remaining arguments
88+
if !is_echo_flag(&arg, &mut echo_options) {
89+
// First non-flag argument stops flag processing
90+
result.push(arg);
91+
break;
92+
}
93+
}
94+
// Collect remaining arguments
95+
for arg in args_iter {
96+
result.push(arg);
97+
}
98+
(result, echo_options.trailing_newline, echo_options.escape)
6199
}
62100

63101
#[uucore::main]
64102
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
65-
let is_posixly_correct = env::var("POSIXLY_CORRECT").is_ok();
103+
// Check POSIX compatibility mode
104+
let is_posixly_correct = env::var_os("POSIXLY_CORRECT").is_some();
66105

106+
let args_iter = args.skip(1);
67107
let (args, trailing_newline, escaped) = if is_posixly_correct {
68-
let mut args_iter = args.skip(1).peekable();
108+
let mut args_iter = args_iter.peekable();
69109

70110
if args_iter.peek() == Some(&OsString::from("-n")) {
71-
let matches = uu_app().get_matches_from(handle_double_hyphens(args_iter));
72-
let args = collect_args(&matches);
111+
// if POSIXLY_CORRECT is set and the first argument is the "-n" flag
112+
// we filter flags normally but 'escaped' is activated nonetheless
113+
let (args, _, _) = filter_echo_flags(args_iter);
73114
(args, false, true)
74115
} else {
75-
let args: Vec<_> = args_iter.collect();
116+
// if POSIXLY_CORRECT is set and the first argument is not the "-n" flag
117+
// we just collect all arguments as every argument is considered an argument
118+
let args: Vec<OsString> = args_iter.collect();
76119
(args, true, true)
77120
}
78121
} else {
79-
let matches = uu_app().get_matches_from(handle_double_hyphens(args.into_iter()));
80-
let trailing_newline = !matches.get_flag(options::NO_NEWLINE);
81-
let escaped = matches.get_flag(options::ENABLE_BACKSLASH_ESCAPE);
82-
let args = collect_args(&matches);
122+
// if POSIXLY_CORRECT is not set we filter the flags normally
123+
let (args, trailing_newline, escaped) = filter_echo_flags(args_iter);
83124
(args, trailing_newline, escaped)
84125
};
85126

tests/by-util/test_echo.rs

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//
33
// For the full copyright and license information, please view the LICENSE
44
// file that was distributed with this source code.
5-
// spell-checker:ignore (words) araba merci mright
5+
// spell-checker:ignore (words) araba merci efjkow
66

77
use uutests::new_ucmd;
88
use uutests::util::TestScenario;
@@ -126,6 +126,16 @@ fn test_escape_override() {
126126
.args(&["-E", "-e", "\\na"])
127127
.succeeds()
128128
.stdout_only("\na\n");
129+
130+
new_ucmd!()
131+
.args(&["-E", "-e", "-n", "\\na"])
132+
.succeeds()
133+
.stdout_only("\na");
134+
135+
new_ucmd!()
136+
.args(&["-e", "-E", "-n", "\\na"])
137+
.succeeds()
138+
.stdout_only("\\na");
129139
}
130140

131141
#[test]
@@ -276,6 +286,89 @@ fn test_double_hyphens_at_start() {
276286
.stdout_only("-- a b --\n");
277287
}
278288

289+
#[test]
290+
fn test_double_hyphens_after_single_hyphen() {
291+
new_ucmd!()
292+
.arg("-")
293+
.arg("--")
294+
.succeeds()
295+
.stdout_only("- --\n");
296+
297+
new_ucmd!()
298+
.arg("-")
299+
.arg("-n")
300+
.arg("--")
301+
.succeeds()
302+
.stdout_only("- -n --\n");
303+
304+
new_ucmd!()
305+
.arg("-n")
306+
.arg("-")
307+
.arg("--")
308+
.succeeds()
309+
.stdout_only("- --");
310+
}
311+
312+
#[test]
313+
fn test_flag_like_arguments_which_are_no_flags() {
314+
new_ucmd!()
315+
.arg("-efjkow")
316+
.arg("--")
317+
.succeeds()
318+
.stdout_only("-efjkow --\n");
319+
320+
new_ucmd!()
321+
.arg("--")
322+
.arg("-efjkow")
323+
.succeeds()
324+
.stdout_only("-- -efjkow\n");
325+
326+
new_ucmd!()
327+
.arg("-efjkow")
328+
.arg("-n")
329+
.arg("--")
330+
.succeeds()
331+
.stdout_only("-efjkow -n --\n");
332+
333+
new_ucmd!()
334+
.arg("-n")
335+
.arg("--")
336+
.arg("-efjkow")
337+
.succeeds()
338+
.stdout_only("-- -efjkow");
339+
}
340+
341+
#[test]
342+
fn test_backslash_n_last_char_in_last_argument() {
343+
new_ucmd!()
344+
.arg("-n")
345+
.arg("-e")
346+
.arg("--")
347+
.arg("foo\n")
348+
.succeeds()
349+
.stdout_only("-- foo\n");
350+
351+
new_ucmd!()
352+
.arg("-e")
353+
.arg("--")
354+
.arg("foo\\n")
355+
.succeeds()
356+
.stdout_only("-- foo\n\n");
357+
358+
new_ucmd!()
359+
.arg("-n")
360+
.arg("--")
361+
.arg("foo\n")
362+
.succeeds()
363+
.stdout_only("-- foo\n");
364+
365+
new_ucmd!()
366+
.arg("--")
367+
.arg("foo\n")
368+
.succeeds()
369+
.stdout_only("-- foo\n\n");
370+
}
371+
279372
#[test]
280373
fn test_double_hyphens_after_flags() {
281374
new_ucmd!()
@@ -292,6 +385,18 @@ fn test_double_hyphens_after_flags() {
292385
.succeeds()
293386
.stdout_only("-- foo\n");
294387

388+
new_ucmd!()
389+
.arg("-ne")
390+
.arg("--")
391+
.succeeds()
392+
.stdout_only("--");
393+
394+
new_ucmd!()
395+
.arg("-neE")
396+
.arg("--")
397+
.succeeds()
398+
.stdout_only("--");
399+
295400
new_ucmd!()
296401
.arg("-e")
297402
.arg("--")

0 commit comments

Comments
 (0)