Subject: Re: [PATCH v2] revision: add separate field for "-m" of "diff-index -m"

From: Junio C Hamano <[email-masked]>
Date: Mon, 31 Aug 2020 10:00:45 -0700

Sergey Organov <[email-masked]> writes:

> $ git diff-index -m --no-diff-merges HEAD
> :100644 000000 4aec621a6d1a9a5892f0b4b6feb2ed329fd04bf2 0000000000000000000000000000000000000000 D	main/main.cc

At the first glance, this looked like a good justification for this
patch.

> If you say "compatibility wart" is not a trouble by itself, -- I'm fine
> with it, -- then "more" in my commit message is misplaced indeed.

Yeah, when I wrote the "compatibility wart" comment originally, I
was describing "this needs a tricky code because two independent
options happen to share the command line parser" and nothing more.

I was not reacting to "more", by the way.  I was reacting the lack
of concrete problem description.  "A '-m' option given to the
'diff-index' command can be defeated by giving '--no-diff-merges'
later" you showed above can be a good replacement for "causes more
troubles".

But in the ideal world, "--[no-]diff-merges" should be rejected as
an irrelevant/unrecognised option to the "diff" family of commands
(as I said in the message you are responding to, it is only relevant
to the "log" family of commands where the diff machinery is solely
to compare between (some of) its parents and in that context, what,
if anything, kind of special treatment is made for merge commits
makes sense as an optional instruction to the command).  Splitting
the field into two fields, setting both fields upon "-m" but
toggling only one with longhand "--[no-]diff-merges" would allow the
code to notice and make the above command line silently turn the
"--[no-]diff-merges" into a no-op, so in that sense it would be a
good first step, but an ideal solution would probably need to know
if we are parsing for the "log" family or for the "diff" family and
error out upon seeing a "log"-only option like "--[no-]diff-merges"
when checking the command line option for "diff".

> This change has nothing to do with defaults. It rather about correct and
> clear code.

OK, I misread your intention.  Sorry about that.

Thanks.