Re: [PATCH 6/6] diff-merges: let -m imply -p

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Alex Henrie <alexhenrie24@xxxxxxxxx> writes:

> On Tue, May 11, 2021 at 12:46 PM Sergey Organov <sorganov@xxxxxxxxx> wrote:
>>
>> Alex Henrie <alexhenrie24@xxxxxxxxx> writes:
>>
>> > On Tue, May 11, 2021 at 8:03 AM Sergey Organov <sorganov@xxxxxxxxx> wrote:
>> >>
>> >> Junio C Hamano <gitster@xxxxxxxxx> writes:
>> >>
>> >> > Junio C Hamano <gitster@xxxxxxxxx> writes:
>> >>
>> >> [...]
>> >>
>> >> > If we enable "some kind of diff" for "-m", I actually think that by
>> >> > default "git log -m" should be turned into "log --cc".  As you told
>> >> > Alex in your response, "log -m -p" is a quite unpleasant format to
>> >> > read---it is there only because it was the only thing we had before
>> >> > we invented "-c/--cc".
>> >>
>> >> Please, no! --cc has unfortunate feature of outputting exactly nothing
>> >> for a lot of merge commits, causing even more confusion than historical
>> >> "-m -p" format.
>> >>
>> >> The best default for -m output is --diff-merges=first-parent. Everybody
>> >> is familiar with it, and it's useful.
>> >>
>> >> > But that might be outside the scope of this series.  I dunno, but if
>> >> > there is no other constraints (like backward compatibility issues),
>> >> > I have a moderately strong preference to use "--cc" over "-m -p"
>> >> > from the get go for unconfigured people, rather than forcing
>> >> > everybody to configure
>> >>
>> >> I rather have strong preference for --diff-merges=first-parent. --cc is
>> >> only suitable for Git experts, and they know how to get what they want
>> >> anyway. Yep, by using --cc. Why spare yet another short option for that?
>> >>
>> >> Overall, let's rather make -m give diff to the first parent by default.
>> >> Simple. Useful. Not confusing.
>> >
>> > Honestly --diff-merges=separate is fine. Two weeks ago, when I started
>> > this discussion, I was trying to use `git log -m` and `git show -m` to
>> > find which merge commit introduced a particular change. Extremely
>> > verbose diff output would have been great for that, the confusing part
>> > was just that `git show -m` produced diff output and `git log -m` did
>> > not.
>>
>> This is not a case in favor of "separate" over "first-parent" as the
>> default for "-m", right?
>>
>> "Which merge commit introduced particular change" is exactly what
>> --diff-merges=1 achieves, so "--diff-merges=separate" was not in fact
>> needed, as I see it. Moreover, it could have produced wrong positives.
>> Looks like --diff-merges=1 is a better fit.
>
> I didn't know which branch the change came from. If the change came
> from the first branch, it would not have appeared under the merge
> commit with --diff-merges=first-parent.

The change in question either came from this merge, or it didn't. If it
came from this merge, it will be there in --diff-merges=first-parent
even if you have octopus merge and multiple parents, in which case it
will be cumulative change from all the side parents.

> But the change would definitely appear with --diff-merges=separate,
> which enabled me to identify the merge commit that included it.

The second part of --diff-merges=separate, that is absent in
--diff-merges=first-parent, shows the diff that this merge would have
made to the side branch, if you had considered the result of merge the
tip of that branch, and you didn't. This second part just makes no sense
at all when you care about changes to your files in any typical git
workflow, as far as I can tell.

> So yes, this is a case in favor of "separate" over "first-parent", but
> it's probably not a common enough scenario to demand keeping
> "separate" for -m.

To me it looks like it rather only shows how deeply confusing
--diff-merges=separate actually is. I'd just kill it.


Thanks,

-- Sergey Organov



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux