From 1a10193e663ff4920964557fd5f0cb6dc0655ee9 Mon Sep 17 00:00:00 2001 From: Konstantin Kharlamov Date: Sat, 21 Jan 2023 12:51:13 +0300 Subject: [PATCH 1/3] cli_args.rs: simplify arguments parser There is no reason for config-handling hooks (like color(), html(), etc) to peek at the value that caused them to be called. So call next() before calling the hooks. That allows to simplify the code and remove some args, but we do not do that per mookid's review, who asked to leave arguments of the hooks to be the same even if some of the functions do not actually use arguments. --- src/cli_args.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/cli_args.rs b/src/cli_args.rs index e3b517b..23eed17 100644 --- a/src/cli_args.rs +++ b/src/cli_args.rs @@ -296,18 +296,20 @@ fn die_error(result: Result) -> bool { } fn color(config: &mut AppConfig, args: &mut Peekable>) -> bool { - let arg = args.next().unwrap(); - if let Some(spec) = args.next() { - die_error(parse_color_arg(&spec, config)) + if let Some(spec) = args.peek() { + let parse_result = parse_color_arg(&spec, config); + args.next(); + die_error(parse_result) } else { - missing_arg(arg) + missing_arg(FLAG_COLOR) } } fn line_numbers(config: &mut AppConfig, args: &mut Peekable>) -> bool { - args.next(); - let spec = if let Some(spec) = args.next() { - parse_line_number_style(config, Some(&*spec)) + let spec = if let Some(spec) = args.peek() { + let parse_result = parse_line_number_style(config, Some(&*spec)); + args.next(); + parse_result } else { parse_line_number_style(config, None) }; @@ -316,13 +318,11 @@ fn line_numbers(config: &mut AppConfig, args: &mut Peekable>) -> bool { config.html = true; - args.next(); true } fn debug(config: &mut AppConfig, args: &mut Peekable>) -> bool { config.debug = true; - args.next(); true } @@ -335,7 +335,7 @@ fn parse_options( config: &mut AppConfig, args: &mut Peekable>, ) -> bool { - if let Some(arg) = args.peek() { + if let Some(arg) = args.next() { match &arg[..] { // generic flags "-h" | "--help" => help(&arg[..] == "--help"), From eebfa0377cee2daa6bd1ee1ff9764567958a5a7b Mon Sep 17 00:00:00 2001 From: Konstantin Kharlamov Date: Sat, 21 Jan 2023 13:23:54 +0300 Subject: [PATCH 2/3] cli_args.rs: handle case of another option following --line-numbers So for example, calling: diffr --line-numbers --help was resulting in error before this commit: unexpected line number style: got '--help', expected aligned|compact Works now. --- src/cli_args.rs | 10 +++++++--- src/tests_cli.rs | 6 ++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/cli_args.rs b/src/cli_args.rs index 23eed17..470dbee 100644 --- a/src/cli_args.rs +++ b/src/cli_args.rs @@ -307,9 +307,13 @@ fn color(config: &mut AppConfig, args: &mut Peekable>) -> bool { let spec = if let Some(spec) = args.peek() { - let parse_result = parse_line_number_style(config, Some(&*spec)); - args.next(); - parse_result + if spec.starts_with("-") { // next option + parse_line_number_style(config, None) + } else { + let parse_result = parse_line_number_style(config, Some(&*spec)); + args.next(); + parse_result + } } else { parse_line_number_style(config, None) }; diff --git a/src/tests_cli.rs b/src/tests_cli.rs index b055936..489c5d8 100644 --- a/src/tests_cli.rs +++ b/src/tests_cli.rs @@ -233,6 +233,12 @@ fn line_numbers_style() { err: Empty, is_success: true, }); + test_cli(ProcessTest { + args: &["--line-numbers", "--line-numbers"], + out: Empty, + err: Empty, + is_success: true, + }); // fail test_cli(ProcessTest { From 050d929aff0741c48297bdc6aef67c6501cf58f8 Mon Sep 17 00:00:00 2001 From: Konstantin Kharlamov Date: Sat, 21 Jan 2023 13:34:55 +0300 Subject: [PATCH 3/3] cli_args.rs: don't rewrap unnecessarily line numbers string The weird call `maybe_line_number_style.map()` is implemented by mookid's review who would not like just changing the parse_line_number_style() argument type. --- src/cli_args.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cli_args.rs b/src/cli_args.rs index 470dbee..879ea27 100644 --- a/src/cli_args.rs +++ b/src/cli_args.rs @@ -306,11 +306,12 @@ fn color(config: &mut AppConfig, args: &mut Peekable>) -> bool { - let spec = if let Some(spec) = args.peek() { + let maybe_line_number_style = args.peek(); + let spec = if let Some(spec) = maybe_line_number_style { if spec.starts_with("-") { // next option parse_line_number_style(config, None) } else { - let parse_result = parse_line_number_style(config, Some(&*spec)); + let parse_result = parse_line_number_style(config, maybe_line_number_style.map(|s| &s[..])); args.next(); parse_result }