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

Add -c short and hide values for --bytes long #48

Merged
merged 3 commits into from Feb 5, 2019

Conversation

selfup
Copy link
Contributor

@selfup selfup commented Feb 3, 2019

c alias for length and -n

References #46

Usage

hexyl doc/logo.svg --c 100

Caveats

So this is interesting. There doesn't seem to be a way to pass in multiple shortnames.

You also can't define the same Arg::with_name("same_name") twice.

potentially seeing if clap would be OK with adding or getting a PR for multiple shortnames would be another option as well

So the closest thing I could get was adding an alias which still means -- instead of -.


Now one thing that could be done would be to add a new Arg block but that feels repetitive?

// support both `--c` as well as `-c`
.arg(
    Arg::with_name("c")
        .short("c")
        .long("c")
        .takes_value(true)
        .value_name("N")
        .help("Read only N bytes from the input"),
)

...

// add new conditional to do the same job as `value_of("length")`
if let Some(count) = matches.value_of("c").and_then(|c| c.parse::<u64>().ok()) {
    reader = Box::new(reader.take(count));
}

https://docs.rs/clap/2.20.0/clap/struct.Arg.html#short

If adding a new --c / -c

The diff would look like (from current commit in PR)

--- a/src/main.rs
+++ b/src/main.rs
@@ -228,7 +228,6 @@ fn run() -> Result<(), Box<::std::error::Error>> {
         .arg(Arg::with_name("file").help("File to display"))
         .arg(
             Arg::with_name("length")
-                .alias("c")
                 .short("n")
                 .long("length")
                 .takes_value(true)
@@ -236,6 +235,14 @@ fn run() -> Result<(), Box<::std::error::Error>> {
                 .help("Read only N bytes from the input"),
         )
         .arg(
+            Arg::with_name("c")
+                .short("c")
+                .long("c")
+                .takes_value(true)
+                .value_name("N")
+                .help("Read only N bytes from the input"),
+        )
+        .arg(
             Arg::with_name("color")
                 .long("color")
                 .takes_value(true)
@@ -264,6 +271,10 @@ fn run() -> Result<(), Box<::std::error::Error>> {
         reader = Box::new(reader.take(length));
     }

+    if let Some(count) = matches.value_of("c").and_then(|c| c.parse::<u64>().ok()) {
+        reader = Box::new(reader.take(count));
+    }
+
     let show_color = match matches.value_of("color") {
         Some("never") => false,
         Some("auto") => atty::is(Stream::Stdout),

@selfup selfup changed the title Add --c alias to length cmd - No support for multiple shortnames Add -c short and hide values for --c long Feb 4, 2019
@selfup
Copy link
Contributor Author

selfup commented Feb 4, 2019

As discussed @sharkdp I have added --c and -c and ensured the values are hidden.

We could always change --c to --count which seems like a viable alias for --length in this context.

However this is the path of least resistance! 🙏

Let me know what you think

Here is a screeny:

hexyl-hidden-command-c

src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@selfup
Copy link
Contributor Author

selfup commented Feb 5, 2019

Ok changed --c to --bytes with a short of -c.

Still hidden from list of commands (maybe it can be shown now that it has valuable meaning?)

--length/-n/-bytes/-c all behave the same now

hexylbyteslength

@selfup selfup changed the title Add -c short and hide values for --c long Add -c short and hide values for --bytes long Feb 5, 2019
@sharkdp sharkdp merged commit 8725f8c into sharkdp:master Feb 5, 2019
@sharkdp
Copy link
Owner

sharkdp commented Feb 5, 2019

Awesome, thank you very much.

@sharkdp
Copy link
Owner

sharkdp commented Feb 5, 2019

Still hidden from list of commands (maybe it can be shown now that it has valuable meaning?)

Sorry. Only saw that after merging.

Yes, we could also make it visible and have the help text say something like "An alias for -n/--length".

@selfup
Copy link
Contributor Author

selfup commented Feb 5, 2019

You are very welcome!

Now I'll go make a new PR to make it visible.

Up! #50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants