Skip to content
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.

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

Argument --length silently takes precedence over --bytes. #105

Closed
eth-p opened this issue Oct 23, 2020 · 2 comments
Closed

Argument --length silently takes precedence over --bytes. #105

eth-p opened this issue Oct 23, 2020 · 2 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@eth-p
Copy link

eth-p commented Oct 23, 2020

When specifying both --length and --bytes in either long or short form, the value of --length will be chosen while the value specified by --bytes will be discarded silently. While I don't think anybody would intentionally run into it, having a warning or error just in case is something that would help improve UX.

hexyl/src/bin/hexyl.rs

Lines 174 to 176 in cc5b308

let mut reader = if let Some(length) = matches
.value_of("length")
.or_else(|| matches.value_of("bytes"))


Note: I'm doing a class assignment regarding creating tests (from scratch) for an open-source command line program, and I chose hexyl for it. You might have a couple more of these on the way, depending on what the assignment brings to light :)

@sharkdp sharkdp added bug Something isn't working good first issue Good for newcomers labels Oct 24, 2020
@sharkdp
Copy link
Owner

sharkdp commented Oct 24, 2020

While I don't think anybody would intentionally run into it, having a warning or error just in case is something that would help improve UX.

That sounds good, yes! We could probably mark them as conflicting (via clap), or as "overrides with".

Note: I'm doing a class assignment regarding creating tests (from scratch) for an open-source command line program, and I chose hexyl for it.

That is an awesome assignment! Thank you for choosing hexyl 👍 😄

@sharkdp
Copy link
Owner

sharkdp commented Oct 24, 2020

By the way: if someone starts implementing integration-style tests for hexyl, I think we should consider using assert_cmd (similar to how we use it for bat).

A test for this would look similar to:

#[test]
fn fail_if_length_and_bytes_options_are_used_simultaneously() {
    hexyl().arg("--length=10").arg("--bytes=5").assert().failure();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants