Re: [PATCH 2/2] format-patch: allow for independent diff & range-diff options

> On Thu, Nov 29 2018, Johannes Schindelin wrote:
> 
> > On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> >
> >> On Thu, Nov 29 2018, Johannes Schindelin wrote:
> >>
> >> > On Thu, 29 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> >> >
> >> >> On Thu, Nov 29 2018, Johannes Schindelin wrote:
> >> >>
> >> >> > On Wed, 28 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> >> >> >
> >> >> >> Change the semantics of the "--range-diff" option so that the regular
> >> >> >> diff options can be provided separately for the range-diff and the
> >> >> >> patch. This allows for supplying e.g. --range-diff-U0 and -U1 to
> >> >> >> "format-patch" to provide different context for the range-diff and the
> >> >> >> patch. This wasn't possible before.
> >> >> >
> >> >> > I really, really dislike the `--range-diff-<random-thing>`. We have
> >> >> > precedent for passing optional arguments that are passed to some other
> >> >> > command, so a much more logical and consistent convention would be to use
> >> >> > `--range-diff[=<diff-option>..]`, allowing all of the diff options that
> >> >> > you might want to pass to the outer diff in one go rather than having a
> >> >> > lengthy string of `--range-diff-this` and `--range-diff-that` options.
> >> >>
> >> >> Where do we pass those sorts of arguments?
> >> >>
> >> >> Reasons I did it this way:
> >> >>
> >> >>  a) Passing it as one option will require the user to double-quote those
> >> >>     options that take quoted arguments (e.g. --word-diff-regex), which I
> >> >>     thought sucked more than the prefix. On the implementation side we
> >> >>     couldn't leave the parsing of the command-line to the shell anymore.
> >> >>
> >> >>  b) I think people will want to tweak this very rarely, much more rarely
> >> >>     than e.g. -U10 in format-patch itself, so having something long-ish
> >> >>     doesn't sound bad.
> >> >
> >> > Hmm. I still don't like it. It sets a precedent, and we simply do not do
> >> > it that way in other circumstances (most obvious would be the -X merge
> >> > options). The more divergent user interfaces for the same sort of thing
> >> > are, the more brain cycles you force users to spend on navigating said
> >> > interfaces.
> >>
> >> Yeah it sucks, I just think it sucks less than the alternative :)
> >> I.e. I'm not picky about --range-diff-* prefix the name, but I think
> >> doing our own shell parsing would be nasty.
> >
> > What prevents you from using `sq_dequote_to_argv()`?
> 
> I mean not just nasty in terms of implementation, yeah we could do it,
> but also a nasty UX for things like --word-diff-regex. I.e. instead of:
> 
>     --range-diff-word-diff-regex='[0-9"]'
> 
> You need:
> 
>     --range-diff-opts="--word-diff-regex='[0-9\"]'"

> Now admittedly that in itself isn't very painful *in this case*, but in
> terms of precedent I really dislike that option, i.e. git having some
> mode where I need to work to escape input to pass to another command.
> 
> Not saying that this --range-diff-* thing is what we should go for, but
> surely we can find some way to do deal with this that doesn't involve
> the user needing to escape stuff like this.
> 
> It also has other downstream effects in the UI, e.g. it's presumably
> easy to teach the bash completion that a --foo=XYZ option is also called
> --some-prefix--foo=XYZ and to enable completion for that, less so for
> making it smart enough to complete "--some-prefix-opts="--foo=<TAB>".