Re: [PATCH 74/76] range-diff: use parse_options() instead of diff_opt_parse()

[Duy Nguyen <pclouds@gmail.com>]

nOn Fri, Jan 18, 2019 at 2:46 AM Stefan Beller <sbeller@google.com> wrote:
> > and we get a
> > looong 'git range-diff -h'
>
> This is an interesting tidbit to put into the commit message.
>
> range-diff is interesting in that in it is unclear where the options
> should take effect. My mental model of range-diff is
>
>     diff --inner-options-1 <range1> >tmp1
>     diff --inner-options-2 <range2> >tmp2
>     diff --outer-options tmp 1 tmp2

This outer/inner refer to the first and second +/- column in
range-diff output, right?

> and for most operations we would want to have the inner
> options to be the same. However there are cases of changing
> one of the inner options, example at
> https://public-inbox.org/git/20180810001010.58870-1-sbeller@google.com/
>
> But even when we assume this to be a corner case for
> weird research of our own options, it is unclear to me
> if the options should apply to the inner diffs or to the
> outer diff or both.
>
> As far as I read the patch, the options are applied to both
> inner and outer, which may be ok?

As far as I can tell, I'm not changing the behavior of this command.
Whatever options accepted before are accepted now. I'm simply exposing
the problem. So no I don't know if it's really ok.

This is not restricted to range-diff either. "git diff" uses
revision.c parser which accepts a whole lot of options that only make
sense with "git log" and friends. Even the "log-tree" command family
has separate set of options for each command, see the "ifdef" in
rev-list-options.txt.

That, I think, would be the next step. To somehow filter options by
command, remove unused ones. Frankly I only have a vague idea how to
do it now ('struct option[]' manipulation).


>
> I would think that sometimes you want to control only the
> inner options, e.g. file copy/rename/move detection
> thresholds. And sometimes you want to control the outer
> options only (white space error highlighting?)



--
Duy