Skip to content

Commit 5238335

Browse files
committed
fix(nl): allow repeated flags to match GNU nl behavior (fixes #9132)
- Add .args_override_self(true) to Command builder to enable 'last wins' behavior - Add .action(ArgAction::Append) to all 10 value-accepting arguments: - BODY_NUMBERING (-b), HEADER_NUMBERING (-h), FOOTER_NUMBERING (-f) - NUMBER_FORMAT (-n), NUMBER_SEPARATOR (-s), NUMBER_WIDTH (-w) - LINE_INCREMENT (-i), JOIN_BLANK_LINES (-l), STARTING_LINE_NUMBER (-v) - SECTION_DELIMITER (-d) - Update helper.rs to use get_last() helper function to retrieve the last value from repeated arguments using get_many() and next_back() - Add 10 regression tests to verify repeated flags are accepted and use last value This matches GNU nl behavior where repeated flags are accepted and the last occurrence's value is used, instead of rejecting with an error.
1 parent 8f7afa7 commit 5238335

File tree

3 files changed

+145
-32
lines changed

3 files changed

+145
-32
lines changed

src/uu/nl/src/helper.rs

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,14 @@ pub fn parse_options(settings: &mut crate::Settings, opts: &clap::ArgMatches) ->
1616
// This vector holds error messages encountered.
1717
let mut errs: Vec<String> = vec![];
1818
settings.renumber = opts.get_flag(options::NO_RENUMBER);
19-
if let Some(delimiter) = opts.get_one::<OsString>(options::SECTION_DELIMITER) {
19+
20+
// Helper function to get the last value from a repeated argument
21+
fn get_last<T: Clone + Send + Sync + 'static>(opts: &clap::ArgMatches, key: &str) -> Option<T> {
22+
opts.get_many::<T>(key)
23+
.and_then(|mut values| values.next_back().cloned())
24+
}
25+
26+
if let Some(delimiter) = get_last::<OsString>(opts, options::SECTION_DELIMITER) {
2027
// GNU nl determines whether a delimiter is a "single character" based on byte length, not
2128
// character length. A "single character" implies the second character is a ':'.
2229
settings.section_delimiter = if delimiter.len() == 1 {
@@ -27,53 +34,49 @@ pub fn parse_options(settings: &mut crate::Settings, opts: &clap::ArgMatches) ->
2734
delimiter.clone()
2835
};
2936
}
30-
if let Some(val) = opts.get_one::<OsString>(options::NUMBER_SEPARATOR) {
31-
settings.number_separator.clone_from(val);
37+
if let Some(val) = get_last::<OsString>(opts, options::NUMBER_SEPARATOR) {
38+
settings.number_separator.clone_from(&val);
3239
}
33-
settings.number_format = opts
34-
.get_one::<String>(options::NUMBER_FORMAT)
40+
settings.number_format = get_last::<String>(opts, options::NUMBER_FORMAT)
3541
.map(Into::into)
3642
.unwrap_or_default();
37-
match opts
38-
.get_one::<String>(options::HEADER_NUMBERING)
39-
.map(String::as_str)
43+
match get_last::<String>(opts, options::HEADER_NUMBERING)
44+
.as_deref()
4045
.map(TryInto::try_into)
4146
{
4247
None => {}
4348
Some(Ok(style)) => settings.header_numbering = style,
4449
Some(Err(message)) => errs.push(message),
4550
}
46-
match opts
47-
.get_one::<String>(options::BODY_NUMBERING)
48-
.map(String::as_str)
51+
match get_last::<String>(opts, options::BODY_NUMBERING)
52+
.as_deref()
4953
.map(TryInto::try_into)
5054
{
5155
None => {}
5256
Some(Ok(style)) => settings.body_numbering = style,
5357
Some(Err(message)) => errs.push(message),
5458
}
55-
match opts
56-
.get_one::<String>(options::FOOTER_NUMBERING)
57-
.map(String::as_str)
59+
match get_last::<String>(opts, options::FOOTER_NUMBERING)
60+
.as_deref()
5861
.map(TryInto::try_into)
5962
{
6063
None => {}
6164
Some(Ok(style)) => settings.footer_numbering = style,
6265
Some(Err(message)) => errs.push(message),
6366
}
64-
match opts.get_one::<usize>(options::NUMBER_WIDTH) {
67+
match get_last::<usize>(opts, options::NUMBER_WIDTH) {
6568
None => {}
66-
Some(num) if *num > 0 => settings.number_width = *num,
69+
Some(num) if num > 0 => settings.number_width = num,
6770
Some(_) => errs.push(translate!("nl-error-invalid-line-width", "value" => "0")),
6871
}
69-
if let Some(num) = opts.get_one::<u64>(options::JOIN_BLANK_LINES) {
70-
settings.join_blank_lines = *num;
72+
if let Some(num) = get_last::<u64>(opts, options::JOIN_BLANK_LINES) {
73+
settings.join_blank_lines = num;
7174
}
72-
if let Some(num) = opts.get_one::<i64>(options::LINE_INCREMENT) {
73-
settings.line_increment = *num;
75+
if let Some(num) = get_last::<i64>(opts, options::LINE_INCREMENT) {
76+
settings.line_increment = num;
7477
}
75-
if let Some(num) = opts.get_one::<i64>(options::STARTING_LINE_NUMBER) {
76-
settings.starting_line_number = *num;
78+
if let Some(num) = get_last::<i64>(opts, options::STARTING_LINE_NUMBER) {
79+
settings.starting_line_number = num;
7780
}
7881
errs
7982
}

src/uu/nl/src/nl.rs

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ pub fn uu_app() -> Command {
245245
.after_help(translate!("nl-after-help"))
246246
.infer_long_args(true)
247247
.disable_help_flag(true)
248+
.args_override_self(true)
248249
.arg(
249250
Arg::new(options::HELP)
250251
.long(options::HELP)
@@ -263,53 +264,60 @@ pub fn uu_app() -> Command {
263264
.short('b')
264265
.long(options::BODY_NUMBERING)
265266
.help(translate!("nl-help-body-numbering"))
266-
.value_name("STYLE"),
267+
.value_name("STYLE")
268+
.action(ArgAction::Append),
267269
)
268270
.arg(
269271
Arg::new(options::SECTION_DELIMITER)
270272
.short('d')
271273
.long(options::SECTION_DELIMITER)
272274
.help(translate!("nl-help-section-delimiter"))
273275
.value_parser(clap::value_parser!(OsString))
274-
.value_name("CC"),
276+
.value_name("CC")
277+
.action(ArgAction::Append),
275278
)
276279
.arg(
277280
Arg::new(options::FOOTER_NUMBERING)
278281
.short('f')
279282
.long(options::FOOTER_NUMBERING)
280283
.help(translate!("nl-help-footer-numbering"))
281-
.value_name("STYLE"),
284+
.value_name("STYLE")
285+
.action(ArgAction::Append),
282286
)
283287
.arg(
284288
Arg::new(options::HEADER_NUMBERING)
285289
.short('h')
286290
.long(options::HEADER_NUMBERING)
287291
.help(translate!("nl-help-header-numbering"))
288-
.value_name("STYLE"),
292+
.value_name("STYLE")
293+
.action(ArgAction::Append),
289294
)
290295
.arg(
291296
Arg::new(options::LINE_INCREMENT)
292297
.short('i')
293298
.long(options::LINE_INCREMENT)
294299
.help(translate!("nl-help-line-increment"))
295300
.value_name("NUMBER")
296-
.value_parser(clap::value_parser!(i64)),
301+
.value_parser(clap::value_parser!(i64))
302+
.action(ArgAction::Append),
297303
)
298304
.arg(
299305
Arg::new(options::JOIN_BLANK_LINES)
300306
.short('l')
301307
.long(options::JOIN_BLANK_LINES)
302308
.help(translate!("nl-help-join-blank-lines"))
303309
.value_name("NUMBER")
304-
.value_parser(clap::value_parser!(u64)),
310+
.value_parser(clap::value_parser!(u64))
311+
.action(ArgAction::Append),
305312
)
306313
.arg(
307314
Arg::new(options::NUMBER_FORMAT)
308315
.short('n')
309316
.long(options::NUMBER_FORMAT)
310317
.help(translate!("nl-help-number-format"))
311318
.value_name("FORMAT")
312-
.value_parser(["ln", "rn", "rz"]),
319+
.value_parser(["ln", "rn", "rz"])
320+
.action(ArgAction::Append),
313321
)
314322
.arg(
315323
Arg::new(options::NO_RENUMBER)
@@ -324,23 +332,26 @@ pub fn uu_app() -> Command {
324332
.long(options::NUMBER_SEPARATOR)
325333
.help(translate!("nl-help-number-separator"))
326334
.value_parser(clap::value_parser!(OsString))
327-
.value_name("STRING"),
335+
.value_name("STRING")
336+
.action(ArgAction::Append),
328337
)
329338
.arg(
330339
Arg::new(options::STARTING_LINE_NUMBER)
331340
.short('v')
332341
.long(options::STARTING_LINE_NUMBER)
333342
.help(translate!("nl-help-starting-line-number"))
334343
.value_name("NUMBER")
335-
.value_parser(clap::value_parser!(i64)),
344+
.value_parser(clap::value_parser!(i64))
345+
.action(ArgAction::Append),
336346
)
337347
.arg(
338348
Arg::new(options::NUMBER_WIDTH)
339349
.short('w')
340350
.long(options::NUMBER_WIDTH)
341351
.help(translate!("nl-help-number-width"))
342352
.value_name("NUMBER")
343-
.value_parser(clap::value_parser!(usize)),
353+
.value_parser(clap::value_parser!(usize))
354+
.action(ArgAction::Append),
344355
)
345356
}
346357

tests/by-util/test_nl.rs

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -800,3 +800,102 @@ fn test_file_with_non_utf8_content() {
800800
String::from_utf8_lossy(invalid_utf8)
801801
));
802802
}
803+
804+
// Regression tests for issue #9132: repeated flags should use last value
805+
#[test]
806+
fn test_repeated_body_numbering_flag() {
807+
// -ba -bt should use -bt (number all lines)
808+
new_ucmd!()
809+
.args(&["-ba", "-bt"])
810+
.pipe_in("a\nb\nc")
811+
.succeeds()
812+
.stdout_is(" 1\ta\n 2\tb\n 3\tc\n");
813+
}
814+
815+
#[test]
816+
fn test_repeated_header_numbering_flag() {
817+
// -ha -ht should use -ht (number all lines in header)
818+
new_ucmd!()
819+
.args(&["-ha", "-ht"])
820+
.pipe_in("a\nb\nc")
821+
.succeeds()
822+
.stdout_is(" 1\ta\n 2\tb\n 3\tc\n");
823+
}
824+
825+
#[test]
826+
fn test_repeated_footer_numbering_flag() {
827+
// -fa -ft should use -ft (number all lines in footer)
828+
new_ucmd!()
829+
.args(&["-fa", "-ft"])
830+
.pipe_in("a\nb\nc")
831+
.succeeds()
832+
.stdout_is(" 1\ta\n 2\tb\n 3\tc\n");
833+
}
834+
835+
#[test]
836+
fn test_repeated_number_format_flag() {
837+
// -n ln -n rn should use -n rn (right-aligned)
838+
new_ucmd!()
839+
.args(&["-n", "ln", "-n", "rn"])
840+
.pipe_in("a\nb\nc")
841+
.succeeds()
842+
.stdout_is(" 1\ta\n 2\tb\n 3\tc\n");
843+
}
844+
845+
#[test]
846+
fn test_repeated_number_separator_flag() {
847+
// -s ':' -s '|' should use -s '|'
848+
new_ucmd!()
849+
.args(&["-s", ":", "-s", "|"])
850+
.pipe_in("a\nb\nc")
851+
.succeeds()
852+
.stdout_is(" 1|a\n 2|b\n 3|c\n");
853+
}
854+
855+
#[test]
856+
fn test_repeated_number_width_flag() {
857+
// -w 3 -w 8 should use -w 8
858+
new_ucmd!()
859+
.args(&["-w", "3", "-w", "8"])
860+
.pipe_in("a\nb\nc")
861+
.succeeds()
862+
.stdout_is(" 1\ta\n 2\tb\n 3\tc\n");
863+
}
864+
865+
#[test]
866+
fn test_repeated_line_increment_flag() {
867+
// -i 1 -i 5 should use -i 5
868+
new_ucmd!()
869+
.args(&["-i", "1", "-i", "5"])
870+
.pipe_in("a\nb\nc")
871+
.succeeds()
872+
.stdout_is(" 1\ta\n 6\tb\n 11\tc\n");
873+
}
874+
875+
#[test]
876+
fn test_repeated_starting_line_number_flag() {
877+
// -v 1 -v 10 should use -v 10
878+
new_ucmd!()
879+
.args(&["-v", "1", "-v", "10"])
880+
.pipe_in("a\nb\nc")
881+
.succeeds()
882+
.stdout_is(" 10\ta\n 11\tb\n 12\tc\n");
883+
}
884+
885+
#[test]
886+
fn test_repeated_join_blank_lines_flag() {
887+
// -l 1 -l 2 should use -l 2 (succeeds, not rejected)
888+
new_ucmd!()
889+
.args(&["-l", "1", "-l", "2"])
890+
.pipe_in("a\nb")
891+
.succeeds();
892+
}
893+
894+
#[test]
895+
fn test_repeated_section_delimiter_flag() {
896+
// -d ':' -d '|' should use -d '|' (succeeds, not rejected)
897+
new_ucmd!()
898+
.args(&["-d", ":", "-d", "|"])
899+
.pipe_in("a\nb")
900+
.succeeds();
901+
}

0 commit comments

Comments
 (0)