sharkdp / hexyl

A command-line hex viewer
Apache License 2.0
8.93k stars 227 forks source link

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

Closed selfup closed 5 years ago

selfup commented 5 years ago

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 commented 5 years ago

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

selfup commented 5 years ago

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

sharkdp commented 5 years ago

Awesome, thank you very much.

sharkdp commented 5 years ago

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 commented 5 years ago

You are very welcome!

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

Up! https://github.com/sharkdp/hexyl/pull/50