Re: [PATCH 0/5] diff-merges: more features

[Sergey Organov <sorganov@gmail.com>]

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

> Sergey Organov <sorganov@gmail.com> writes:
>
>> 2. log.diffMerges-m-imply-p
>>
>> Historically, '-m' doesn't imply '-p' whereas similar '-c' and '--cc'
>> options do. Simply fixing this inconsistency by unconditional
>> modification of '-m' semantics appeared to be a bad idea, as it broke
>> some legacy scripts/aliases. This patch rather provides configuration
>> variable to tweak '-m' behavior accordingly.
>
> I do not know how this can be a good idea.  For those users who set
> the configuration variables, those scripts and aliases get broken
> anyway, don't they?  IOW, I am not sure how this is better than the
> "modification of '-m'" that is "a bad idea".

The history behind this is that before the patch #1 of these series
there was no way to get exact semantics of current '-m' option using new
--diff-merges option, so there was no sensible way to "fix" these
scripts/aliases. That was in fact the primary objection for the new '-m'
semantics.

Now, when one stomps on such script/alias (by explicitly enabling the
new configuration option), they can fix it by replacing '-m' with
'--diff-merges=on,hide', that, along with the last patch of these series
that produces warning when lone '-m' is detected, looks to me as a way
to eventually get rid of the legacy and surprising '-m' semantics.

> I would understand why it may be a safer and more sensible solution,
> if the proposed approach were to find an unused letter $X and to
> introduce "-$X" that is the same as "-m" but implies "-p", though.

I've in fact considered this as well, and '-d' was free for such use
last time I've checked. It was very tempting to just add '-d' that
always shows diff to first parent for everything, be it merge or not,
and call it a day, but it won't fix '-m', which inconsistent behavior
still surprises people, and besides started the whole --diff-merges
business in the first place.

>
>> 3. log.diffMergesForce
>>
>> Force specific log format for -c, --cc, and --remerge-diff options
>> instead of their respective formats. The override is useful when some
>> external tool hard-codes diff for merges format option.
>
> Not convinced it is a good idea for the same reason as above (not
> convinced it is a bad idea, either, though).

Here the intention was entirely different though. I'm using magit and it
does hard-code --cc in one place that I'd like to be able to override.
If I got this problem, I figured chances are high somebody else will get
it as well, and that's the rationale.

Then I figure everybody here has own favorite format for merge commits,
and this toy gives you exactly this: whatever option is used (once), you
get what you prefer instead, or use an option second time to enforce it.

For stable scripting, we for rather long time now have --diff-merges=c,
and --diff-merges=cc, as well as slightly more recent
--diff-merges=remerge, that are not affected by the configuration in
question.

That said, it looks useful to me, but I won't insist on the feature
should you guys object.

>
>> 4. Support list of values for --diff-merges
>>
>> This allows for shorter --diff-merges=on,hide forms.
>
> Good, probably.

Was useless before 'hide' though, as the rest of options just override
each other entirely.

Thanks,
-- Sergey Organov