Skip to content

Cleanups and fixes to command parsing #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Cleanups and fixes to command parsing #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1a10193
cli_args.rs: simplify arguments parser
Hi-Angel Jan 21, 2023
eebfa03
cli_args.rs: handle case of another option following --line-numbers
Hi-Angel Jan 21, 2023
050d929
cli_args.rs: don't rewrap unnecessarily line numbers string
Hi-Angel Jan 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions src/cli_args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,18 +296,25 @@ fn die_error<TRes>(result: Result<TRes, ArgParsingError>) -> bool {
}

fn color(config: &mut AppConfig, args: &mut Peekable<impl Iterator<Item = String>>) -> 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<impl Iterator<Item = String>>) -> bool {
args.next();
let spec = if let Some(spec) = args.next() {
parse_line_number_style(config, Some(&*spec))
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, maybe_line_number_style.map(|s| &s[..]));
args.next();
parse_result
}
} else {
parse_line_number_style(config, None)
};
Expand All @@ -316,13 +323,11 @@ fn line_numbers(config: &mut AppConfig, args: &mut Peekable<impl Iterator<Item =

fn html(config: &mut AppConfig, args: &mut Peekable<impl Iterator<Item = String>>) -> bool {
config.html = true;
args.next();
true
}

fn debug(config: &mut AppConfig, args: &mut Peekable<impl Iterator<Item = String>>) -> bool {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keep the signatures of all the arguments handlers the same (debug, html, line_numbers...) even if the functions themselves look dull.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Odd, but you are the boss. Do you want me to leave that as is with the warning of args being unused, or is there any way to silence it? Like a pragma or what analog does Rust have

config.debug = true;
args.next();
true
}

Expand All @@ -335,7 +340,7 @@ fn parse_options(
config: &mut AppConfig,
args: &mut Peekable<impl Iterator<Item = String>>,
) -> bool {
if let Some(arg) = args.peek() {
if let Some(arg) = args.next() {
match &arg[..] {
// generic flags
"-h" | "--help" => help(&arg[..] == "--help"),
Expand Down
6 changes: 6 additions & 0 deletions src/tests_cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down