Re: [PATCH 74/76] range-diff: use parse_options() instead of diff_opt_parse()
- From
Duy Nguyen <pclouds@gmail.com>
- Date
- Jan 18, 2019, 09:30 UTC
- Message-ID
- <CACsJy8A7SpCnXHfYg=Tx4kyUwONbAMufWkhuV4pfyu+BdFbWmQ@mail.gmail.com>
- In-Reply-To
- <CAGZ79kaAf7NytateigG_f6-UbeJKa2bno9+4zC7R4uqfFB77FA@mail.gmail.com>
nOn Fri, Jan 18, 2019 at 2:46 AM Stefan Beller <sbeller@google.com> wrote:
Show 11 quoted lines
> > 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?
Show 12 quoted lines
> 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).
Show 5 quoted lines
> > 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