Ability to pass a diff/patch file as single argument about riff

[walles]

How about this? This is what I use.

riff < hello.diff

[0x5c]

How about this? This is what I use.

riff < hello.diff

Way less "natural" than even piping from cat, at least for me. It's a "pattern" I encounter/use very rarely, contrary to cat <file> | cmd, cmd <file>, and cmd -f <file>

[walles]

I think having an option for this would be fine.

Maybe --file or -f as you hinted at.

I'd slightly prefer having the --file form only, to lower the risk of colliding with any plain diff options. But neither macOS diff nor GNU diff has any short -f option so I guess just -f should be fine as well.

Wanna make a PR?

[0x5c]

Wanna make a PR?

sure I'll have a look

[0x5c]

So I finally had the time to have a look, but with the way the custom argument parser is done it's going to be painful to add a flag that takes an option.

[0x5c]

In this dive in the code, I found a funny bug

consume() is so over-eager that it deletes the second file from argv.

Additionally, it would normally be possible to end option/flag parsing with -- like in the touch calls, but riff does not support that

I would recommend switching to an argument parsing crate like clap, both to ease extensibility, to avoid argument parsing edge-cases like these, to provide proper CLI validation, and to follow argument parsing conventions and best practices like --. The structure of the rest of the code would make that an easy switch. I also offer to make a PR for that.

[walles]

I started looking into that here:
https://github.com/walles/riff/commits/johan/clap

It mostly works, but Clap supports only one Usage description which I find unfortunate. Not sure what to do with that...

[0x5c]

It mostly works, but Clap supports only one Usage description which I find unfortunate. Not sure what to do with that...

Ah yeah clap's default is not the best. You can override the usage line(s) with override_usage and it supports multiple lines

To use it with the derive, you just add override_usage = USAGE here

riff/src/main.rs

Line 59 in d1c5c71

#[command(version = GIT_VERSION, name = "riff", about = "Colors diff output, highlighting the changed parts of every line.", after_help = HELP_TEXT_FOOTER)]

[0x5c]

Successfully added it back

diff --git a/src/main.rs b/src/main.rs
index 97c818e..6964892 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -29,6 +29,10 @@ mod refiner;
 mod token_collector;
 mod tokenizer;
 
+const USAGE: &str = "diff ... | riff\n       \
+    riff [-b] [--no-pager] <file1> <file2>\n       \
+    riff [-b] [--no-pager] <directory1> <directory2>";
+
 const HELP_TEXT_FOOTER: &str = r#"Installing riff in the $PATH:
     sudo cp riff /usr/local/bin
 
@@ -56,7 +60,7 @@ const PAGER_FORKBOMB_STOP: &str = "_RIFF_IGNORE_PAGER";
 const GIT_VERSION: &str = git_version!(cargo_prefix = "");
 
 #[derive(Parser)]
-#[command(version = GIT_VERSION, name = "riff", about = "Colors diff output, highlighting the changed parts of every line.", after_help = HELP_TEXT_FOOTER)]
+#[command(version = GIT_VERSION, name = "riff", about = "Colors diff output, highlighting the changed parts of every line.", after_help = HELP_TEXT_FOOTER, override_usage = USAGE)]
 struct Options {
     /// First file to compare
     #[arg(requires("file2"))]

file1 and file2 could be Option<PathBuf> to automatically get a valid path, and the check for the paths being file-file or dir-dir could be done earlier since the paths would already be paths from the get-go.

[walles]

Branch merged, thanks for the help!

If you see something that can be adjusted do let me know.

Now it should be rather straight forward to add that new option!

[0x5c]

If you see something that can be adjusted do let me know.

The lack of the seven spaces on the usage lines gives an odd rendering to the usage lines, since they normally start on the same line as Usage: in most cli tools, but that's just styling.

[walles]

The lack of the seven spaces on the usage lines gives an odd rendering to the usage lines, since they normally start on the same line as Usage: in most cli tools, but that's just styling.

Arguments and Options are indented by two, so I'll stick to that for Usage as well.

As you say, this is a matter of taste...