Junio C Hamano <gitster@xxxxxxxxx> writes:
> Sergey Organov <sorganov@xxxxxxxxx> writes:
>
>>> I thought I already said this, but in case I didn't, I think
>>> "--diff-merges=separate" should imply "some kind of diff", and not
>>> necessarily "-p".
>>
>> Is this a more polite way to say "no"? If not, how is it relevant for
>> -m, now being a synonym for --diff-merges=on?
>
> Sorry, I didn't mean to say "no" to anything.
To me "no" is as good answer as any, I just want to reach better
understanding.
>
> I wrote 'separate' not because I wanted to special case that (and
> treat others like 'on' differently), but simply because I didn't
> want to write "--diff-merges=<anything>" as "off/no" should not
> imply "show some kind of diff".
>
>> As for particular idea, I'll repeat myself as well and say that I'm
>> still against implying anything by any off --diff-merges, and even more
>> against implying something that affects non-merge commits. --diff-merges
>> are not convenience options that need to be short yet give specific
>> functionality, so there is no place for additional implications.
>
> So -m (a shorthand for --diff-merges=on) should not imply any patch
> generation, you mean?
No, I don't mean it. The idea is to let -m be alias for
"--diff-merges=on -p", exactly the same way --cc is currently
essentially an alias for "--diff-merges=dense-combined -p".
What I meant is that --diff-merges itself should not imply any patch
generation for non-merge commits, so --diff-merges=on should not imply
-p.
> It matches what we seem to have agreed on to be the purist view in a
> few messages ago. --diff-merges controls which parent(s) comparison is
> made against in a merge, -p/--cc/--raw/--stat etc. control how the
> result of that comparison is expressed.
I see this as your vision, but I don't recall we agree on it. At least
that's not how it currently works, as far as I can tell, see command
examples I gathered in one of my previous answers.
>
> But I also remember that we agreed that the purist view design was
> cumbersome to use, so --diff-merges=<anything but no> implying "show
> some kind of diff" is OK, plus if nobody says "what kind" via the
> command line with -p/--cc/--raw/--stat etc., it is OK to default to
> '-p'.
The latter part of this sentence is something rather new to me, that
only appeared in this particular thread of discussion recently, and it
does not match my own vision. Neither my vision of the current
implementation nor of what we should aim for.
>
> One thing I think our unnecessary "disagreement" comes from is that
> among "-m", "--cc", "-c", you say "-m" is the only thing that does
> not imply "-p", but I do not view "--cc" and "-c" as sitting next to
> "-m" at all in the first place.
I was sure you rather did when we've discussed it the last time before
this thread. Now your opinion seems to have changed, and I don't see
why. In fact I'm very confused. As far as understood, that time you said
that -m has been simply overlooked when --cc/-c started to imply -p, and
that you actually don't care about -m that much anyway.
I also recall you said that -c (and later --cc) has been invented as
alternative to not that useful -m, so -m, -c, and -cc have always been
exactly sitting next to each other in my view.
> "-m" is on the "which parent(s) to compare with" side,
This has never been the case, has it? See:
-m
This flag makes the merge commits show the full diff like regular
commits; for each merge parent, a separate log entry and diff is
generated. An exception is that only diff against the first parent
is shown when --first-parent option is given; in that case, the
output represents the changes the merge brought into the
then-current branch.
-c
With this option, diff output for a merge commit shows the
differences from each of the parents to the merge result
simultaneously instead of showing pairwise diff between a parent
and the result one at a time. Furthermore, it lists only files
which were modified from all parents.
First, -m doesn't select the parents at all and shows "full diff", so it
rather defines the format. And second, -c is described exactly as being
alternative format to -m, as far as I can tell, making -c sit right next
to -m again, contrary to what you say above.
BTW, I recall I once suggested something like what you said, let -m
match what in means in cherry-pick, to what parent(s) to compare, but
it'd need -m to take (optional) argument(s), that has been considered
unacceptable, so the idea has been rejected (and for the better.)
> while "--cc" and "-c" are "now you decided which parent(s) to compare
> with, how does the result of comparison presented?" side. And because
> "--cc"/"-c" explicitly wants to work on merge commits (because it
> naturally degenerates to simple "--patch" for non merges), THEY are
> made to imply "-m" (i.e. compare with all parents).
That's a reasonable interpretation. The problem is that currently this
does not match nor design, nor implementation, nor documentation at all,
as far as I can tell.
> So from my point of view, "--cc/-c" implying "-m" has no relevance
> to whether "-m" should or should not imply "some kind of comparison
> should be shown".
What you describe is a different design that may well be a good one, but
do we actually want to change what's already there? What for?
>
> But because we agreed that we want to bend the purist view for
> usability and included cc/c among the choices diff-merges=<choice>
> can take, I think -m (but not log.diffMerges=no case) should imply
> "we should show some kind of patch".
Once again, this doesn't fit into the current design, as far as I can
tell, or I misunderstand the design, that could well be the case as
well.
>
> Which would mean that unless when log.diffMerges or --diff-merges
> say off/no, and unless there is any option to specify how the result
> of comparison should bepresented on the command line:
>
> - when log.diffMerges or --diff-merges say cc or c, default to --cc
> or -c.
>
> - otherwise,default to --patch.
>
> is what I think should happen. But the reason I think so is not
> because "--cc" and "-c" gives output without "-m" (i.e. "-p" does
> not imply "-m" and it should not).
I don't like this so far. Considering -m to be just one of different
formats to represent merge commits (among -c and --cc), as it has always
been, looks more straightforward and useful to me.
Besides, all the recent design I authored assumed -m to be just that,
one of multiple ways to specify how to represent merge commits, the
other 2 being -c and --cc. If we decide to change this view, it'd likely
need significant re-design, and I'm yet to see any actual advantages.
If, on the other hand, it's just me who fundamentally misunderstands the
design, then I need to be corrected fast, before I make significant
damage.
Thanks,
-- Sergey Organov
