Re: so/diff-merges-more (was Re: What's cooking in git.git (Feb 2023, #01; Thu, 2))

From: Sergey Organov <sorganov@gmail.com>
Date: Mar 25, 2023, 16:55 UTC
Message-ID: <87y1nk6aa5.fsf@osv.gnss.ru>
In-Reply-To: <xmqq8rfugbuy.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 17 quoted lines

> Sergey Organov <sorganov@gmail.com> writes:
>
>>> I do not quite understand the last one (#4),
>>
>> Well, -m does not imply -p, whereas the rest of diff-merges options
>> (-c/--cc/--remerge-diff) do imply -p. This is what half of this
>> lengthy discussion was about.
>>
>>> own 4., it would be that introducing --diff-merges={kind} may have
>>> been a mistake.  It would have been fine and better to just let
>>> users choose from whatever set of options we support, i.e. (-c,
>>> --cc, --remerge-diff, -m -p, -m --raw, ...).
>
>> It's fine with me that --cc is everything you need, but what I need is
>> rather diff to the first parent, ...
>
> I think "show --first-parent" should give that already.

Well, for "git show" even "show -m" does the right thing (once properly configured), as "-p" is implied by "git show".

Taking "git show" into the picture brings yet another argument if favor of new "-m" behavior, as then "git show -m" and "git log -m -n1" will finally start to produce the same result, that I'd find desirable.

That said,

  --diff-merges=first-parent

that could be shortened to

  --diff-merges=1

is the universal answer that works out-of-the-box for any command the same way, reliably, and then it's also

  -m -p

if configured accordingly, that has been made available by previously accepted patches.

These series just did the last logical step: allowed it to be just

-m

if configured accordingly.

> One problem with "-m implies -p" is that it is unclear what should be
> done to things like "-m --raw".

Nothing specific is actually needed, as far as I'm aware, as implied -p doesn't interfere with --raw. Please give particular example of a problem if you foresee one.

As I see it, if there is indeed some problem with this, it should already exist for -c, --cc, and --remerge-diff, and then probably needs to be fixed anyway. Moreover, it should also exist for "git show", as the latter implies -p, and there is:

       -s, --no-patch
           Suppress diff output. Useful for commands like git show that show
           the patch by default, or to cancel the effect of --patch.

[As a side-note, current behavior of implied -p, explicit -p, -s, and --raw with respect to each other that I figured by experiment looks suspect to me. E.g., once explicit -p is given, and then canceled by -s, I can't get bare --raw output anymore]

> Yes, we can declare an arbitrary choice (like "-m implies -p unless
> --raw, --stat, etc. are given") but that is just replacing an
> arbitrary rule [...] with another one.

Uh, this would be too cumbersome indeed, but fortunately it does not seem to be needed, see above.

Overall, letting -m imply -p just makes things more consistent, even on the problems side (if any), and I honestly still don't see'em.

Thanks, -- Sergey Organov