Generalize delta subcommands: rg, diff (implicit), show
The possible command line now is this:
delta <delta-args> [SUBCMD <subcmd-args>]
If the entire command line fails to parse because SUBCMD is unknown, then try parsing <delta-args> only, and on success call SUBCMD and pipe input to delta.
Available are:
delta rg .. => rg --json .. | delta
delta show .. => git show --color=always .. | delta
delta a b .. => diff a b .. | delta
The piping is not done by the shell, but delta, so the subcommands now are child processes of delta
Looking at the Call { Delta, .. } enum from the --help PR, and how the diff sub-command works (both how -@ is needed to add extra args, and how its ouput is fed back into delta when running as a child process) I had the idea of generalizing that.
Deltas rg integration is great, but rg --json .. | delta is a bit clunky, now it is just delta rg ... This also allows using delta without touching any git configs ("try before you edit anything"), as a proof of concept delta show is implemented here.
This is a bit ... creative with the clap command line parser, but it is all quite contained.
Nice, very interesting!
- I'm sure you considered this but is supporting
delta git $subcommanda possibility also? - Are there any parse ambiguity challenges, e.g. if someone tries to diff a file named
rgetc?
Some miscellaneous comments about subcommands; not sure how relevant to this PR:
- there are various existing options that perhaps should be subcommands themselves, such as
--show-colors,--show-config,--show-syntax-themes,--show-themes,--parse-ansi. Ought we to consider making those subcommands before 1.0? - Would be nice to have auto-generated shell completion for all this (I believe the trend nowadays is for that to be another subcommand, i.e.
eval $(delta completion bash)Use https://crates.io/crates/clap_complete?). - Also maybe
eval $(delta toggle side-by-side)andeval $(delta toggle line-numbers)mutating$DELTA_FEATURES?
I'm sure you considered this but is supporting
delta git $subcommanda possibility also?
I used --color=always, which is not accepted by all subcommands - but by calling git -c color.ui=always .. any subcommand should become callable. Just check if git-$subcommand is in $PATH.
Are there any parse ambiguity challenges, e.g. if someone tries to diff a file named
rgetc?
Yes, and in that case I would give priority to the subcommand. By using ./rg this can ways be overwritten.
Some miscellaneous comments about subcommands; not sure how relevant to this PR:
there are various existing options that perhaps should be subcommands themselves, such as
--show-colors,--show-config,--show-syntax-themes,--show-themes,--parse-ansi. Ought we to consider making those subcommands before 1.0?
I think these are not used that often, I would leave them as flags (these being located in src/subcommands/ notwithstanding).
Would be nice to have auto-generated shell completion for all this (I believe the trend nowadays is for that to be another subcommand, i.e.
eval $(delta completion bash)Use https://crates.io/crates/clap_complete?).
Already present, see --generate-completion :) But that could also become a subcommand like that.
Also maybe
eval $(delta toggle side-by-side)andeval $(delta toggle line-numbers)mutating$DELTA_FEATURES?
Sure, should be possible as well.
I'm sure you considered this but is supporting
delta git $subcommanda possibility also?
Now also possible!
However enabling all git-X commands directly would mean checking if "git $X" is a valid git subcommand first (while parsing the command line, repeatedly), so I use a specific list:
pub const SUBCOMMANDS: &[&str] = &[RG, "show", "log", "diff", "grep", "blame", GIT];
Are there any other useful commands? Also, can you check why the rg test fails on macOS?
"show", "log", "diff", "grep", "blame" Are there any other useful commands?
The ones that are coming to mind are add for git add -p, and reflog and stash for git reflog -p and git stash list -p.
Also, can you check why the rg test fails on macOS?
It fails because of https://github.com/dandavison/delta/blob/acb7c1294397c8c2ccd1a68c786261ea6dc22dd3/src/subcommands/generic_subcmd.rs#L127
https://docs.rs/grep-cli/latest/grep_cli/fn.resolve_binary.html
On non-Windows, this is a no-op.
This API has always seemed very surprising to me. I've opened https://github.com/BurntSushi/ripgrep/issues/2928 to ask about it and maybe make the docstring clearer.
I use zsh and have eval "$(delta --generate-completion zsh 2>/dev/null)" in my shell config. It looks like there might be a shell completion challenge relating to this PR: normally, a command line like rg pattern xxx<TAB> completes xxx as a file path. But delta rg pattern xxx<TAB> is not doing that.
The ones that are coming to mind are
addforgit add -p, andreflogandstashforgit reflog -pandgit stash list -p.
Done.
[The rg test fails on macOS fail] because of https://docs.rs/grep-cli/latest/grep_cli/fn.resolve_binary.html
Oh, thanks! Never would have suspected that, re-implemented some of try_resolve_binary() (which is not public!) in an almost-one-liner for the test.
shell completion
Indeed, I have also asked myself that for simpler stuff like writing my own git-foo command, the completion has to embed itself into the existing git completion.
The ones that are coming to mind are
addforgit add -p, andreflogandstashforgit reflog -pandgit stash list -p.
And one more: I recently learned of the existence of checkout -p (from https://github.com/dandavison/delta/issues/1908)
there are various existing options that perhaps should be subcommands themselves, such as --show-colors, --show-config, --show-syntax-themes, --show-themes, --parse-ansi. Ought we to consider making those subcommands before 1.0?
I think these are not used that often, I would leave them as flags (these being located in src/subcommands/ notwithstanding).
Also maybe eval $(delta toggle side-by-side) and eval $(delta toggle line-numbers) mutating $DELTA_FEATURES?
Sure, should be possible as well.
Just to mention one more -- I'm attracted to adding a "doctor" command that users can run to obtain a standard suite of diagnostics describing their environment, something like the direction started in https://github.com/dandavison/delta/pull/1193. Would something like that fit with the work in this branch as a delta doctor subcommand with doctor-specific arguments?
checkout -p
Added.
The doctor or other subcommands can be added via clap directly, or --show-color can be become a show-color subcommand if you prefer that. These here are external (renamed it to that!) subcommands which have to side-step clap and pipe their output back into the parent process.
This is slightly off-topic because I think this would be an internal rather than an external subcommand, but I just found myself doing delta /dev/null path/to/myfile in order to "cat" a file but with delta's hyperlinked line numbers (which I have set up to open in my IDE). So perhaps that suggests delta cat might be another entry in our subcommand namespace, with the green background color disabled, and the minus line number column removed.
Apart from delta rg, the other commands mostly make trying out delta easier. But yes, they could just as well sit behind git for that. And I am not even against the redundant diff command, because well, a diff is a diff, and a delta is something not-quite-a-diff. But whatever you prefer.
I sometimes try to use delta 0123.patch, which doesn't work, until I remember delta < 0123.patch also does the job. Maybe default to something like that for a single argument?
Apart from delta rg, the other commands mostly make trying out delta easier. But yes, they could just as well sit behind git for that.
Let's make them sit behind git for now. We can then watch how our subcommand namespace evolves, and of course it would always be possible to revisit the decision in the future.
Done.
This is untested on Windows, but the process logic is from subcommands/diff.rs so I don't expect surprises so feel free to merge.
Apologies if the problem is at my end but I think that something might have gone wrong in the last change? When I do delta git show xxx it looks like the show is being popped out of the command, yielding git -c color.ui=always xxx. I'm guessing related to that double "git" thing we were discussing above?
No, you are perfectly right! Fixed, and not just testing rg but also git now. That was some hasty last-minute refactoring. Refactored that part again, the logic is much cleaner now.
Ah, the test doesn't work because the CI runners do a shallow --depth=1 clone, fix. And a new clippy with Rust v1.83, with a false positive zombie warning...
Excellent, merged!
Both git and ripgrep have special behavior when their output is redirected, aimed at creating predictable and parseable output when it's being consumed by a machine. Should we consider not invoking delta at all in that case, instead execing the external command?
Excellent, merged!
Booo, squash merged again :)
You envision someone doing alias rg='delta rg', which would break on rg foo | rg bar? But then delta also needs the equivalent of --color=always|auto|none, but for terminal detection, so someone still can do delta rg > pretty-result.ansi.txt.
Booo, squash merged again :)
Oh I'm sorry, that is annoying. The button is usually defaulted to squash since we use it for other delta contributors so it is a bit of an easy mistake for me to make, especially because we only ever squash at work, so my brain sees "Squash and merge" as "This is the green merge button"... But perhaps now I will not forget again.
But then delta also needs the equivalent of --color=always|auto|none, but for terminal detection, so someone still can do delta rg > pretty-result.ansi.txt.
Perhaps, but I think that I might have a good counterargument to that: today, if a user wants to save delta output with ANSI escapes, they have to do git foo | delta > pretty-result.ansi.txt because git itself has tty detection that doesn't even invoke $GIT_PAGER when redirected. And since saving pretty colored output to disk is unusual, I don't think it has to be particularly ergonomic. So I think in fact it might make sense for us to keep a consistent rule that always, with delta, if you want to save pretty output to disk, then pipe explicitly to delta.
For rg, a user would have to supply --json explicitly: rg --json | delta > pretty-result.ansi.txt, or delta rg --json | delta > pretty-result.ansi.txt. So there'd be a bit of abstraction leakage since it kind of requires them to know that delta's rg integration is based on rg --json. But, at the end of the day, I do think "delta output is for humans" is a reasonable project design principle and that people that are saving ANSI escape codes in text files (or parsing them) probably can be trusted to figure out the details?
Our git workflow uses Gerrit and is heavily based on "stacked PRs" (as it is now called) - so exactly the opposite, and clearly superior ;) No worries, I'll just leave multi-commit PRs as a draft/WIP and do the final merge myself.
Agreed, the direct exec subcommand PR is #1925, without an opt-out. Let's see if someone runs into that and comes up with another argument for --color=always.