git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: Why doesn't `git log -m` imply `-p`?
       [not found] <CANvKV6anT0MV7GDeY_Cd2r+BwvMjsTdmjK+s_DKth7ZqL0XRUQ@mail.gmail.com>
@ 2021-05-19 17:03 ` Sergey Organov
  0 siblings, 0 replies; 28+ messages in thread
From: Sergey Organov @ 2021-05-19 17:03 UTC (permalink / raw)
  To: huang guanlong; +Cc: gitster, alexhenrie24, git


1. It's -m that does now imply -p, in "git log", in "git show", etc.,
wherever it's used...

2. Manual says "-c implies -p", "--cc implies -p", and now "-m implies
-p".

-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-10 12:11 ` Sergey Organov
@ 2021-05-10 16:56   ` Alex Henrie
  0 siblings, 0 replies; 28+ messages in thread
From: Alex Henrie @ 2021-05-10 16:56 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Git mailing list, Junio C Hamano

On Mon, May 10, 2021 at 6:11 AM Sergey Organov <sorganov@gmail.com> wrote:
>
> Alex Henrie <alexhenrie24@gmail.com> writes:
>
> > I read the following in `man git-log` today:
> >
> > --diff-merges=separate, --diff-merges=m, -m
> >     This makes merge commits show the full diff with respect to each of
> >     the parents. Separate log entry and diff is generated for each
> >     parent.  -m doesn't produce any output without -p.
> >
> > --diff-merges=combined, --diff-merges=c, -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.  -c implies -p.
> >
> > --diff-merges=dense-combined, --diff-merges=cc, --cc
> >     With this option the output produced by --diff-merges=combined is
> >     further compressed by omitting uninteresting hunks whose contents
> >     in the parents have only two variants and the merge result picks one
> >     of them without modification.  --cc implies -p.
> >
> > Why do -c and -cc imply -p, but -m does not? I tried to use both `git
> > log -c` and `git log -m` today and was confused when the latter didn't
> > produce any output. Could we change this behavior in a future version
> > of Git?
>
> Patches to fix this are almost ready, but I'd like to make a warning
> that you'd likely be even more confused by the current output of "-m",
> unless you set "log.diffMerges" configuration option to "first-parent".
> These diffs with respect to /second/ parent at the end of the output
> made me really mad once upon a time.

Good observation. I'm not sure what would be the ideal default here,
but certainly having -m turn on /some/ kind of diff is better than
nothing. Thanks!

-Alex

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-29  1:44 Alex Henrie
  2021-04-29  3:22 ` Junio C Hamano
@ 2021-05-10 12:11 ` Sergey Organov
  2021-05-10 16:56   ` Alex Henrie
  1 sibling, 1 reply; 28+ messages in thread
From: Sergey Organov @ 2021-05-10 12:11 UTC (permalink / raw)
  To: Alex Henrie; +Cc: Git mailing list, Junio C Hamano

Alex Henrie <alexhenrie24@gmail.com> writes:

> I read the following in `man git-log` today:
>
> --diff-merges=separate, --diff-merges=m, -m
>     This makes merge commits show the full diff with respect to each of
>     the parents. Separate log entry and diff is generated for each
>     parent.  -m doesn't produce any output without -p.
>
> --diff-merges=combined, --diff-merges=c, -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.  -c implies -p.
>
> --diff-merges=dense-combined, --diff-merges=cc, --cc
>     With this option the output produced by --diff-merges=combined is
>     further compressed by omitting uninteresting hunks whose contents
>     in the parents have only two variants and the merge result picks one
>     of them without modification.  --cc implies -p.
>
> Why do -c and -cc imply -p, but -m does not? I tried to use both `git
> log -c` and `git log -m` today and was confused when the latter didn't
> produce any output. Could we change this behavior in a future version
> of Git?

Patches to fix this are almost ready, but I'd like to make a warning
that you'd likely be even more confused by the current output of "-m",
unless you set "log.diffMerges" configuration option to "first-parent".
These diffs with respect to /second/ parent at the end of the output
made me really mad once upon a time.

-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-06 20:48                                 ` Sergey Organov
@ 2021-05-07  1:31                                   ` Alex Henrie
  0 siblings, 0 replies; 28+ messages in thread
From: Alex Henrie @ 2021-05-07  1:31 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Junio C Hamano, Git mailing list

On Thu, May 6, 2021 at 2:48 PM Sergey Organov <sorganov@gmail.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> F> Sergey Organov <sorganov@gmail.com> writes:
> >
> >> It's nice we've reached mutual understanding!
> >
> > Yes, and thanks for correcting me.
> >
> >> The only remaining issue then is if we just go and do the change of -m
> >> semantics, or do we need to take some backward compatibility measures?
> >> Looks like we are rather safe to just go, as it's unlikely there will be
> >> any real breakage. What do you think?
> >
> > I still wish I could come up with the usual backward compatibility
> > transition dance for this case, but I do not think there is one.
>
> Fine, thanks, so I'll prepare and submit a patch.

Thanks guys! I am so glad you were able to reach a consensus!

-Alex

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-06 20:29                               ` Junio C Hamano
@ 2021-05-06 20:48                                 ` Sergey Organov
  2021-05-07  1:31                                   ` Alex Henrie
  0 siblings, 1 reply; 28+ messages in thread
From: Sergey Organov @ 2021-05-06 20:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Henrie, Git mailing list

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

F> Sergey Organov <sorganov@gmail.com> writes:
>
>> It's nice we've reached mutual understanding!
>
> Yes, and thanks for correcting me.
>
>> The only remaining issue then is if we just go and do the change of -m
>> semantics, or do we need to take some backward compatibility measures?
>> Looks like we are rather safe to just go, as it's unlikely there will be
>> any real breakage. What do you think?
>
> I still wish I could come up with the usual backward compatibility
> transition dance for this case, but I do not think there is one.

Fine, thanks, so I'll prepare and submit a patch.

>
> However.
>
> If "-m" were doing a more useful thing than "compare with each
> parent separately", people may have aliased "log -m" to something so
> that their "git aliased-log" and "git aliased-log -p" would work
> better for them than "git log" and "git log -p", but quite honestly,
> I do not think "git log -m -p" output is readable by humans (after
> all, that is why we invented -c and --cc), so the population that
> get hit by this incompatible change may be very tiny minority in
> relative terms.

Well, honestly, I can't even come up with an alias that would break by
this change, but it's likely I'm just not creative enough :)

Thanks,
-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-06 12:59                             ` Sergey Organov
@ 2021-05-06 20:29                               ` Junio C Hamano
  2021-05-06 20:48                                 ` Sergey Organov
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2021-05-06 20:29 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Alex Henrie, Git mailing list

Sergey Organov <sorganov@gmail.com> writes:

> It's nice we've reached mutual understanding!

Yes, and thanks for correcting me.

> The only remaining issue then is if we just go and do the change of -m
> semantics, or do we need to take some backward compatibility measures?
> Looks like we are rather safe to just go, as it's unlikely there will be
> any real breakage. What do you think?

I still wish I could come up with the usual backward compatibility
transition dance for this case, but I do not think there is one.

However.

If "-m" were doing a more useful thing than "compare with each
parent separately", people may have aliased "log -m" to something so
that their "git aliased-log" and "git aliased-log -p" would work
better for them than "git log" and "git log -p", but quite honestly,
I do not think "git log -m -p" output is readable by humans (after
all, that is why we invented -c and --cc), so the population that
get hit by this incompatible change may be very tiny minority in
relative terms.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-06  0:27                           ` Junio C Hamano
@ 2021-05-06 12:59                             ` Sergey Organov
  2021-05-06 20:29                               ` Junio C Hamano
  0 siblings, 1 reply; 28+ messages in thread
From: Sergey Organov @ 2021-05-06 12:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Henrie, Git mailing list

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

> Sergey Organov <sorganov@gmail.com> writes:
>
>> No, I don't mean it. The idea is to let -m be alias for
>> "--diff-merges=on -p",...
>
> Ahhhh, that makes a whole lot of difference.  Thanks.
>
>> 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.
>
> No, it was I who was confused, as I somehow incorrectly thoguht that
> your plan was to make "-m" identical to "--diff-merges=on".
>
> But if your plan is to make
>
>     "git log -m"    (no other option)
>     "git log -m -p"
>
> behave identically to "git log --diff-merges=on -p",

Yep, exactly.

> and similarly make
>
>     "git log -m --stat"
>     "git log -m --raw"
>
> behave identically to "git log --diff-merges=on --stat/--raw", I
> think that such a design makes quite a lot of sense.

These I honestly didn't even think about before, but now, after you've
brought them, I'll pay attention, thanks!

>
> It will still keep the purity of "--diff-merges=<choice>" (that is,
> it only is about if/how a merge is expressed in some form of diff),
> while solving the longstanding usability issue of "-m" that led to
> Alex's "when a user says -m, diff output is expected", that came
> quite early in this thread.

It's nice we've reached mutual understanding!

The only remaining issue then is if we just go and do the change of -m
semantics, or do we need to take some backward compatibility measures?
Looks like we are rather safe to just go, as it's unlikely there will be
any real breakage. What do you think?

Thanks,
-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-05 13:43                         ` Sergey Organov
@ 2021-05-06  0:27                           ` Junio C Hamano
  2021-05-06 12:59                             ` Sergey Organov
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2021-05-06  0:27 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Alex Henrie, Git mailing list

Sergey Organov <sorganov@gmail.com> writes:

> No, I don't mean it. The idea is to let -m be alias for
> "--diff-merges=on -p",...

Ahhhh, that makes a whole lot of difference.  Thanks.

> 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.

No, it was I who was confused, as I somehow incorrectly thoguht that
your plan was to make "-m" identical to "--diff-merges=on".

But if your plan is to make

    "git log -m"    (no other option)
    "git log -m -p"

behave identically to "git log --diff-merges=on -p", and similarly
make

    "git log -m --stat"
    "git log -m --raw"

behave identically to "git log --diff-merges=on --stat/--raw", I
think that such a design makes quite a lot of sense.

It will still keep the purity of "--diff-merges=<choice>" (that is,
it only is about if/how a merge is expressed in some form of diff),
while solving the longstanding usability issue of "-m" that led to
Alex's "when a user says -m, diff output is expected", that came
quite early in this thread.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-05  0:20                       ` Junio C Hamano
@ 2021-05-05 13:43                         ` Sergey Organov
  2021-05-06  0:27                           ` Junio C Hamano
  0 siblings, 1 reply; 28+ messages in thread
From: Sergey Organov @ 2021-05-05 13:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Henrie, Git mailing list

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

> Sergey Organov <sorganov@gmail.com> 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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-04 14:18                     ` Sergey Organov
@ 2021-05-05  0:20                       ` Junio C Hamano
  2021-05-05 13:43                         ` Sergey Organov
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2021-05-05  0:20 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Alex Henrie, Git mailing list

Sergey Organov <sorganov@gmail.com> 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.  

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?  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.

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'.

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.

"-m" is on the "which parent(s) to compare with" side, 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).

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".

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".

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).

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-04 20:09       ` Felipe Contreras
@ 2021-05-04 20:34         ` Sergey Organov
  0 siblings, 0 replies; 28+ messages in thread
From: Sergey Organov @ 2021-05-04 20:34 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Alex Henrie, Junio C Hamano, Git mailing list

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Alex Henrie wrote:
>> On Wed, Apr 28, 2021 at 9:22 PM Junio C Hamano <gitster@pobox.com> wrote:
>> > Alex Henrie <alexhenrie24@gmail.com> writes:
>
>> > It is best to move on, writing it off as historical accident, and
>> > embrace the new --diff-merges=m option, instead of wasting time on
>> > pondering "why", because accidents do not have to have a deep reason
>> > behind them ;-)
>> 
>> If the behavior is an idiosyncratic accident of dubious utility, let's
>> replace it with something that makes sense and is useful :-) If we
>> make -m imply -p then no alias is necessary, `git log` would display
>> the log without diffs and `git log -m` would display the log with all
>> the diffs.
>
> Indeed. Mistakes in the design of the UI should not be carried on
> forever.
>
> Either all --diff-merges={m,c,cc} imply -p, or none should.

None of --diff-merges imply -p, and none should, -- that's one of the
goals of introducing --diff-merges.

OTOH, -c/--cc do imply -p, and then -m doesn't. That's the historical
inconsistency, not anything about recently implemented --diff-merges,
and the natural way to fix it is to let -m imply -p.

Thanks,

-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-29 15:25     ` Alex Henrie
  2021-04-29 16:35       ` Sergey Organov
@ 2021-05-04 20:09       ` Felipe Contreras
  2021-05-04 20:34         ` Sergey Organov
  1 sibling, 1 reply; 28+ messages in thread
From: Felipe Contreras @ 2021-05-04 20:09 UTC (permalink / raw)
  To: Alex Henrie, Sergey Organov; +Cc: Junio C Hamano, Git mailing list

Alex Henrie wrote:
> On Wed, Apr 28, 2021 at 9:22 PM Junio C Hamano <gitster@pobox.com> wrote:
> > Alex Henrie <alexhenrie24@gmail.com> writes:

> > It is best to move on, writing it off as historical accident, and
> > embrace the new --diff-merges=m option, instead of wasting time on
> > pondering "why", because accidents do not have to have a deep reason
> > behind them ;-)
> 
> If the behavior is an idiosyncratic accident of dubious utility, let's
> replace it with something that makes sense and is useful :-) If we
> make -m imply -p then no alias is necessary, `git log` would display
> the log without diffs and `git log -m` would display the log with all
> the diffs.

Indeed. Mistakes in the design of the UI should not be carried on
forever.

Either all --diff-merges={m,c,cc} imply -p, or none should.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-04 12:38                   ` Junio C Hamano
@ 2021-05-04 14:18                     ` Sergey Organov
  2021-05-05  0:20                       ` Junio C Hamano
  0 siblings, 1 reply; 28+ messages in thread
From: Sergey Organov @ 2021-05-04 14:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Henrie, Git mailing list

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

> Sergey Organov <sorganov@gmail.com> writes:
>
>> That said, what do we decide about -m to finally join -c/--cc party and
>> start to imply -p? Last time we've discussed it, we decided that -m has
>> been simply overlooked when -c/--cc started to imply -p. Should we
>> finally fix this?
>
> 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?

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.

That said, I think that something like your idea could be fine if we
introduce another convenience option, say, -d, that will imply both
--diff-merges=separate and "some kind of diff" (whatever the latter
actually means, I'm not sure yet.) But then again, why don't just reuse
-m that, as we've decided before, is not that useful in its current
state anyway?

I must admit that I don't entirely understand your idea above yet. Maybe
you could provide a draft of manual entry for proposed behavior of
--diff-merges=separate, for better understanding? For convenience, right
now it reads:

   --diff-merges=separate
       This makes merge commits show the full diff with respect to
       each of the parents. Separate log entry and diff is generated
       for each parent.

Thanks,

-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-04  9:10                 ` Sergey Organov
@ 2021-05-04 12:38                   ` Junio C Hamano
  2021-05-04 14:18                     ` Sergey Organov
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2021-05-04 12:38 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Alex Henrie, Git mailing list

Sergey Organov <sorganov@gmail.com> writes:

> That said, what do we decide about -m to finally join -c/--cc party and
> start to imply -p? Last time we've discussed it, we decided that -m has
> been simply overlooked when -c/--cc started to imply -p. Should we
> finally fix this?

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".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-04  1:15               ` Junio C Hamano
@ 2021-05-04  9:10                 ` Sergey Organov
  2021-05-04 12:38                   ` Junio C Hamano
  0 siblings, 1 reply; 28+ messages in thread
From: Sergey Organov @ 2021-05-04  9:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Henrie, Git mailing list

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

> Sergey Organov <sorganov@gmail.com> writes:
>
>>>  * "--patch", "--stat", "--cc" etc are to specify if we use the diff
>>>    machinery and what kind of output is desired.
>>
>> So, in your view, --cc output is not a product of "diff machinery"?
>
> I view --cc and -c as an enhanced form of --patch that is also
> capable of handling multi-way diffs, in other words, choosing --cc
> should be to say "give me textual patch for all commits; when there
> are multiple parents, condense multi-way patches".
>
> So, yes, strictly speaking, --diff-merges=cc was probably a mistake,
> and in the ideal world, --diff-merges should have taken only one of
> "compare with nothing" (optional), "compare with first-parent", and
> "compare with all parents".  The last choice could output diffs in
> various forms, like traditional -m (i.e. patch output separately for
> each parent), --cc, -c, etc.  "compare with nothing" is optional
> because we could also control on the "output format" side to say
> "produce no output" (ala "git show -s").
>
> But such an idealized orthogonal design without special casing will
> often lead to usability problems and complaints that -m alone does
> not produce anything, so I am OK to have cc and friends as the value
> for --diff-merges for that reason.

I basically agree with what you say here, and it's in fact very close to
the first variant of the design that originally came to my mind, and it
was reality that shifted things to the way they are implemented now.

That said, what do we decide about -m to finally join -c/--cc party and
start to imply -p? Last time we've discussed it, we decided that -m has
been simply overlooked when -c/--cc started to imply -p. Should we
finally fix this?


Thanks,

-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-03 17:42             ` Sergey Organov
@ 2021-05-04  1:15               ` Junio C Hamano
  2021-05-04  9:10                 ` Sergey Organov
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2021-05-04  1:15 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Alex Henrie, Git mailing list

Sergey Organov <sorganov@gmail.com> writes:

>>  * "--patch", "--stat", "--cc" etc are to specify if we use the diff
>>    machinery and what kind of output is desired.
>
> So, in your view, --cc output is not a product of "diff machinery"?

I view --cc and -c as an enhanced form of --patch that is also
capable of handling multi-way diffs, in other words, choosing --cc
should be to say "give me textual patch for all commits; when there
are multiple parents, condense multi-way patches".

So, yes, strictly speaking, --diff-merges=cc was probably a mistake,
and in the ideal world, --diff-merges should have taken only one of
"compare with nothing" (optional), "compare with first-parent", and
"compare with all parents".  The last choice could output diffs in
various forms, like traditional -m (i.e. patch output separately for
each parent), --cc, -c, etc.  "compare with nothing" is optional
because we could also control on the "output format" side to say
"produce no output" (ala "git show -s").

But such an idealized orthogonal design without special casing will
often lead to usability problems and complaints that -m alone does
not produce anything, so I am OK to have cc and friends as the value
for --diff-merges for that reason.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-05-01  0:41           ` Junio C Hamano
@ 2021-05-03 17:42             ` Sergey Organov
  2021-05-04  1:15               ` Junio C Hamano
  0 siblings, 1 reply; 28+ messages in thread
From: Sergey Organov @ 2021-05-03 17:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Henrie, Git mailing list

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

> Sergey Organov <sorganov@gmail.com> writes:
>
>>> Luckily,
>>>
>>>     $ git log [--stat] --diff-merges=first-parent master..seen
>>>
>>> seems to do almost the right thing, with respect to the "It is
>>> probably OK to special case" I gave above.
>>
>> I believe any special-casing is to be a last resort, and definitely is
>> not the right thing to do in this particular case.
>
> I do not know if I get it.  "log --diff-merges=<kind>" giving the
> same output as "log" (i.e. no trace of any kind of diff) would be
> puzzling to users, and to help them, it is OK to say that

I thought (apparently wrong) that the idea was to special-case "-m", and
only "-m". I.e., if -m is alone, let it imply -p, otherwise not. That
was the thing I was in opposition to.

>
>  * "--diff-merges=<kind>" enables some kind of diff output
>    automatially (for both merges and non-merges), and

No, I don't think this is OK, sorry. I fail to see why --diff-merges
should affect non-merge commits. I believe it shouldn't.

>
>  * when there is no user preference given as to what kind of diff is
>    desired, we default to "-p".

What kind of diff "-p" gives for merge commits, exactly? As far as I can
tell, it's "none".

>
> As it is natural to expect "--stat --diff-merges=<kind> would give
> only the diffstat without patch, we end up "special casing"
> "--diff-merges=<kind>" that is given alone, without specifying what
> kind of diff is desired, and behave as if "-p" was given.

In general, I hate dependencies between options. The only thing that is
actually needed for convenience is an ability for an option to imply
other options. What's currently there is already too complex for my
personal taste, and I'd hate to add even more complexity on top.

Right now --diff-merges is pretty simple and straightforward: it
specifies diff output for merge commits, and only for merge commits. If
any other option disables diff machinery altogether, it will disable
diff for merges as well.

OTOH, we have --patch that deals with non-merge commits.

Personally, I don't like the resulting interface very much, but it's
historical, and can't be easily changed.

> So I would have expected you to call this kind of "special casing" a
> good thing.

Well, even though I was originally commenting about -m only, I must
admit I'm in general against any "special casing", unless there is
extremely strong reason to consider it.

>
>>> It only "enables diff" for merge commits, which does not quite feel
>>> right and we may want to do the same "enable diff" for single parent
>>> commits,

I fail to see why --diff-merges should ever affect non-merge commits.
It'd be at least counter-intuitive, not to say directly opposite to the
original design goal.

>>> but the good part is that it does not blindly imply "-p".

Yep.

>>>
>>> It seems to do the "enable diff" the right way by honoring other
>>> command line options that specify the format of the diff, so with
>>> "--stat" included in the sample command line above, we get the
>>> diffstat for single parent commits (because we ask for "--stat" from
>>> the command line to show it throughout the history) and also for
>>> merge commits (because --diff-merges=first-parent does *not* blindly
>>> turn the textual patch '-p' on).
>>
>> Good to know! I must admit I did nothing special in this regard, just
>> paid attention to avoid breaking any existing logic, at least knowingly.
>>
>>>
>>>> [Footnote]
>>>>
>>>> *1* They are not limited to "-p", "--stat" and "--summary", but
>>>> you'd need to also pay attention to "--raw", "--name-only", etc.)
>>>
>>> I've merged the so/log-diff-merge topic to 'master', with this
>>> (possibly) known breakage that it does not do anything for single
>>> parent commits.  We may want to fix this last mile before the
>>> release that is scheduled to happen around early June.
>>
>> I have no idea what the breakage is or could be.
>
> Because I view
>
>  * "--diff-merges" is a way to specify how merge commits are passed
>    to the diff machinery (e.g. pass nothing to the diff machinery,
>    compare only with the first parent, etc.), and

As I see it, it only defines the way they are to be represented by the
diff machinery once passed to it, though it obviously depends on where
we put the margin of "diff machinery".

>
>  * "--patch", "--stat", "--cc" etc are to specify if we use the diff
>    machinery and what kind of output is desired.

So, in your view, --cc output is not a product of "diff machinery"?

> but we are conflating the "enable diff" feature into the former to
> match end-user expectation, if "--diff-merges" without any of the
> "--patch", "--stat", etc. enables the "--patch" output for merge
> commits, it would be confusing if we do not give the same "--patch"
> output for single-parent commits, too.
>
> But the current code gives "--patch" output only for merge commits,
> doesn't it?  E.g.

No, as far as I understand it, "--patch" output is for non-merge commits
only. One can't sensibly use patch utility to pick merge commits anyway,
so "--patch" makes no sense for merge commits and doesn't affect them,
at least for now.

>
>     $ git log --diff-merges=first-parent master..next
>
> would give patches only for merge commits, but

It will give the output similar to what "--patch" would give for
non-merge commits, yes, but in fact it's not "--patch" output, I think,
so I doubt it should be called "give patches". It's just happens to be
the same diff format.

>
>     $ git log --stat --diff-merges=first-parent master..next
>
> would give us diffstat for all commits, including merges (against
> their first parents).

Yep, but I think it just matches the old behavior that has been always
there, see below.

I'd start from the behavior even before my patches. Let's see:

  git log -n1 -p <merge_commit>
  git log -n1 --stat <merge_commit>
  git log -n1 --stat -p <merge_commit>

all give no diff no stat. No surprise for diff, though not that sure about
stat, but it could be argued either way.

  git log -n1 -c <merge_commit>

does give diff output in particular format, nice!

  git log -n1 --stat -c <merge_commit>

gives stat output, but no diff! That's not what I expected at all.
Effectively, this looks like --stat *disables* -c/-cc output?

Finally, the way to get both diff and stat for merge commits is... who'd
guess, adding -p to the command, and that provided -p is already
supposedly implied by -c (!):

  git log -n1 --stat -c -p <merge_commit>

In particular, this means that contrary to documentation, -c does not
imply -p in the common sense of the word "imply", and interdependencies
between all these options are already too complex to easily grok for a
human being.

As for newer --diff-merges, they behave similar to -c here that seems
reasonable. Overall, I still don't see any breakage introduced by
--diff-merges, and it seems to behave according to its documentation, so
shouldn't break any expectations either.

Getting back to the original question of letting -m imply -p, it
shouldn't behave differently than -c/-cc, that do imply -p, so I don't
see any significant problem that'd be added to the current status.

Right now the following two give exactly the same output:

  git log -n1 --stat -c <merge_commit>
  git log -n1 --stat -m <merge_commit>

the stat to the first parent, and it shouldn't change if we let -m "imply"
-p the same way -c "implies" -p, whatever it actually means.

Best Regards,

-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-30 14:00         ` Sergey Organov
@ 2021-05-01  0:41           ` Junio C Hamano
  2021-05-03 17:42             ` Sergey Organov
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2021-05-01  0:41 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Alex Henrie, Git mailing list

Sergey Organov <sorganov@gmail.com> writes:

>> Luckily,
>>
>>     $ git log [--stat] --diff-merges=first-parent master..seen
>>
>> seems to do almost the right thing, with respect to the "It is
>> probably OK to special case" I gave above.
>
> I believe any special-casing is to be a last resort, and definitely is
> not the right thing to do in this particular case.

I do not know if I get it.  "log --diff-merges=<kind>" giving the
same output as "log" (i.e. no trace of any kind of diff) would be
puzzling to users, and to help them, it is OK to say that

 * "--diff-merges=<kind>" enables some kind of diff output
   automatially (for both merges and non-merges), and

 * when there is no user preference given as to what kind of diff is
   desired, we default to "-p".

As it is natural to expect "--stat --diff-merges=<kind> would give
only the diffstat without patch, we end up "special casing"
"--diff-merges=<kind>" that is given alone, without specifying what
kind of diff is desired, and behave as if "-p" was given.  So I
would have expected you to call this kind of "special casing" a good
thing.

>> It only "enables diff" for merge commits, which does not quite feel
>> right and we may want to do the same "enable diff" for single parent
>> commits, but the good part is that it does not blindly imply "-p".
>>
>> It seems to do the "enable diff" the right way by honoring other
>> command line options that specify the format of the diff, so with
>> "--stat" included in the sample command line above, we get the
>> diffstat for single parent commits (because we ask for "--stat" from
>> the command line to show it throughout the history) and also for
>> merge commits (because --diff-merges=first-parent does *not* blindly
>> turn the textual patch '-p' on).
>
> Good to know! I must admit I did nothing special in this regard, just
> paid attention to avoid breaking any existing logic, at least knowingly.
>
>>
>>> [Footnote]
>>>
>>> *1* They are not limited to "-p", "--stat" and "--summary", but
>>> you'd need to also pay attention to "--raw", "--name-only", etc.)
>>
>> I've merged the so/log-diff-merge topic to 'master', with this
>> (possibly) known breakage that it does not do anything for single
>> parent commits.  We may want to fix this last mile before the
>> release that is scheduled to happen around early June.
>
> I have no idea what the breakage is or could be.

Because I view

 * "--diff-merges" is a way to specify how merge commits are passed
   to the diff machinery (e.g. pass nothing to the diff machinery,
   compare only with the first parent, etc.), and

 * "--patch", "--stat", "--cc" etc are to specify if we use the diff
   machinery and what kind of output is desired.

but we are conflating the "enable diff" feature into the former to
match end-user expectation, if "--diff-merges" without any of the
"--patch", "--stat", etc. enables the "--patch" output for merge
commits, it would be confusing if we do not give the same "--patch"
output for single-parent commits, too.

But the current code gives "--patch" output only for merge commits,
doesn't it?  E.g.

    $ git log --diff-merges=first-parent master..next

would give patches only for merge commits, but

    $ git log --stat --diff-merges=first-parent master..next

would give us diffstat for all commits, including merges (against
their first parents).

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-30  4:50       ` Junio C Hamano
@ 2021-04-30 14:00         ` Sergey Organov
  2021-05-01  0:41           ` Junio C Hamano
  0 siblings, 1 reply; 28+ messages in thread
From: Sergey Organov @ 2021-04-30 14:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Henrie, Git mailing list

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

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Sergey Organov <sorganov@gmail.com> writes:
>>
>>> Yep, but --diff-merges=m doesn't imply -p either, though it does produce
>>> diff output without -p, for merge commits only.
>>
>> I misspoke without thinking it through.  It is absolutely wrong for
>> the "-m" option (or "--diff-merges=m" for that matter) to imply
>> "-p".
>>
>> $ git log --stat --summary
>>
>> would show "diff", but the kind of "diff" requested is not a textual
>> patch but just diffstat and the summary of new/removed files, and
>> the "diff" is shown only for single-parent commits, and it omits
>> "diff" for merge commits.  Adding "-m" to this command line is *not*
>> a request to show the textual patch.  It is to ask "diff" to be
>> shown pairwise with each of the parent.
>>
>> $ git log -m --stat --summary
>>
>> It is probably OK to special case "-m" given alone without any other
>> option [*1*] that specifies what kind of "diff" is requested and
>> make it imply "-p".  But unconditionally flipping "-p" on only
>> because you saw "-m" (or "--diff-merges=m" for that matter) is just
>> wrong.
>
> Luckily,
>
>     $ git log [--stat] --diff-merges=first-parent master..seen
>
> seems to do almost the right thing, with respect to the "It is
> probably OK to special case" I gave above.

I believe any special-casing is to be a last resort, and definitely is
not the right thing to do in this particular case.

>
> It only "enables diff" for merge commits, which does not quite feel
> right and we may want to do the same "enable diff" for single parent
> commits, but the good part is that it does not blindly imply "-p".
>
> It seems to do the "enable diff" the right way by honoring other
> command line options that specify the format of the diff, so with
> "--stat" included in the sample command line above, we get the
> diffstat for single parent commits (because we ask for "--stat" from
> the command line to show it throughout the history) and also for
> merge commits (because --diff-merges=first-parent does *not* blindly
> turn the textual patch '-p' on).

Good to know! I must admit I did nothing special in this regard, just
paid attention to avoid breaking any existing logic, at least knowingly.

>
>> [Footnote]
>>
>> *1* They are not limited to "-p", "--stat" and "--summary", but
>> you'd need to also pay attention to "--raw", "--name-only", etc.)
>
> I've merged the so/log-diff-merge topic to 'master', with this
> (possibly) known breakage that it does not do anything for single
> parent commits.  We may want to fix this last mile before the
> release that is scheduled to happen around early June.

I have no idea what the breakage is or could be. Do we have any relevant
tests in place already, or can somebody suggest some to check for
possible breakage?

> Note that I didn't check if you are doing the right thing for all
> formats, or if I was lucky and --stat was only one of them you paid
> attention to, when you needed to notice all others that you don't.
> But if you used the same logic that allows "git show" to by default
> give "-p/--cc" output while "git show --stat" to squelch the patch
> output, you should be OK.

In fact I didn't do anything specific to --stat, nor to other options
you mention (--raw, --name-only, --summary), so I'd expect all of them
still work the same way they were before my --diff-merges series. Need
to be checked anyway, sure thing, and that gets us back to the questions
of tests. I personally don't know what expectations are, so it's hard
for me to implement needed tests.

Thanks,
-- Sergey Organov


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-29 23:27     ` Junio C Hamano
@ 2021-04-30  4:50       ` Junio C Hamano
  2021-04-30 14:00         ` Sergey Organov
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2021-04-30  4:50 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Alex Henrie, Git mailing list

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

> Sergey Organov <sorganov@gmail.com> writes:
>
>> Yep, but --diff-merges=m doesn't imply -p either, though it does produce
>> diff output without -p, for merge commits only.
>
> I misspoke without thinking it through.  It is absolutely wrong for
> the "-m" option (or "--diff-merges=m" for that matter) to imply
> "-p".
>
> $ git log --stat --summary
>
> would show "diff", but the kind of "diff" requested is not a textual
> patch but just diffstat and the summary of new/removed files, and
> the "diff" is shown only for single-parent commits, and it omits
> "diff" for merge commits.  Adding "-m" to this command line is *not*
> a request to show the textual patch.  It is to ask "diff" to be
> shown pairwise with each of the parent.
>
> $ git log -m --stat --summary
>
> It is probably OK to special case "-m" given alone without any other
> option [*1*] that specifies what kind of "diff" is requested and
> make it imply "-p".  But unconditionally flipping "-p" on only
> because you saw "-m" (or "--diff-merges=m" for that matter) is just
> wrong.

Luckily,

    $ git log [--stat] --diff-merges=first-parent master..seen

seems to do almost the right thing, with respect to the "It is
probably OK to special case" I gave above.

It only "enables diff" for merge commits, which does not quite feel
right and we may want to do the same "enable diff" for single parent
commits, but the good part is that it does not blindly imply "-p".

It seems to do the "enable diff" the right way by honoring other
command line options that specify the format of the diff, so with
"--stat" included in the sample command line above, we get the
diffstat for single parent commits (because we ask for "--stat" from
the command line to show it throughout the history) and also for
merge commits (because --diff-merges=first-parent does *not* blindly
turn the textual patch '-p' on).

> [Footnote]
>
> *1* They are not limited to "-p", "--stat" and "--summary", but
> you'd need to also pay attention to "--raw", "--name-only", etc.)

I've merged the so/log-diff-merge topic to 'master', with this
(possibly) known breakage that it does not do anything for single
parent commits.  We may want to fix this last mile before the
release that is scheduled to happen around early June.

Note that I didn't check if you are doing the right thing for all
formats, or if I was lucky and --stat was only one of them you paid
attention to, when you needed to notice all others that you don't.
But if you used the same logic that allows "git show" to by default
give "-p/--cc" output while "git show --stat" to squelch the patch
output, you should be OK.

Thanks.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-29 12:38   ` Sergey Organov
  2021-04-29 15:25     ` Alex Henrie
@ 2021-04-29 23:27     ` Junio C Hamano
  2021-04-30  4:50       ` Junio C Hamano
  1 sibling, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2021-04-29 23:27 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Alex Henrie, Git mailing list

Sergey Organov <sorganov@gmail.com> writes:

> Yep, but --diff-merges=m doesn't imply -p either, though it does produce
> diff output without -p, for merge commits only.

I misspoke without thinking it through.  It is absolutely wrong for
the "-m" option (or "--diff-merges=m" for that matter) to imply
"-p".

$ git log --stat --summary

would show "diff", but the kind of "diff" requested is not a textual
patch but just diffstat and the summary of new/removed files, and
the "diff" is shown only for single-parent commits, and it omits
"diff" for merge commits.  Adding "-m" to this command line is *not*
a request to show the textual patch.  It is to ask "diff" to be
shown pairwise with each of the parent.

$ git log -m --stat --summary

It is probably OK to special case "-m" given alone without any other
option [*1*] that specifies what kind of "diff" is requested and
make it imply "-p".  But unconditionally flipping "-p" on only
because you saw "-m" (or "--diff-merges=m" for that matter) is just
wrong.


[Footnote]

*1* They are not limited to "-p", "--stat" and "--summary", but
you'd need to also pay attention to "--raw", "--name-only", etc.)

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-29 17:24         ` Alex Henrie
@ 2021-04-29 19:07           ` Sergey Organov
  0 siblings, 0 replies; 28+ messages in thread
From: Sergey Organov @ 2021-04-29 19:07 UTC (permalink / raw)
  To: Alex Henrie; +Cc: Junio C Hamano, Git mailing list

Alex Henrie <alexhenrie24@gmail.com> writes:

> On Thu, Apr 29, 2021 at 10:35 AM Sergey Organov <sorganov@gmail.com> wrote:
>>
>> Alex Henrie <alexhenrie24@gmail.com> writes:
>>
>> > On Thu, Apr 29, 2021 at 6:38 AM Sergey Organov <sorganov@gmail.com> wrote:
>> >>
>> >> As the final purpose of all this is to have -m as user-friendly short
>> >> option, I'd incline to finally let it imply -p, as --diff-merges=m now
>> >> covers another side of the coin.
>> >>
>> >> What do you think?
>> >
>> > I am 100% in favor of that proposal, and I can work on the code this
>> > weekend.
>>
>> No need to bother. If we agree, I'll send a patch atop of my recent
>> changes that make -m format configurable.
>
> Great, thank you!
>
>> Alternatively, we can add a configuration option, or let -m imply -p
>> only when -m format is explicitly configured by the user.
>
> Since the goal here is simple, easily understandable, and
> user-friendly behavior, I think -m should imply -p all the time, or at
> least imply -p by default. The less I have to explain to new Git
> users, the better.

Yep, but OTOH -m never implied -p before, and it'll take time for the
change get to release and then to reach distributions... So the actual
question here is if anybody cares enough about backward compatibility in
this particular case to complicate transition?

-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-29 16:35       ` Sergey Organov
@ 2021-04-29 17:24         ` Alex Henrie
  2021-04-29 19:07           ` Sergey Organov
  0 siblings, 1 reply; 28+ messages in thread
From: Alex Henrie @ 2021-04-29 17:24 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Junio C Hamano, Git mailing list

On Thu, Apr 29, 2021 at 10:35 AM Sergey Organov <sorganov@gmail.com> wrote:
>
> Alex Henrie <alexhenrie24@gmail.com> writes:
>
> > On Thu, Apr 29, 2021 at 6:38 AM Sergey Organov <sorganov@gmail.com> wrote:
> >>
> >> As the final purpose of all this is to have -m as user-friendly short
> >> option, I'd incline to finally let it imply -p, as --diff-merges=m now
> >> covers another side of the coin.
> >>
> >> What do you think?
> >
> > I am 100% in favor of that proposal, and I can work on the code this
> > weekend.
>
> No need to bother. If we agree, I'll send a patch atop of my recent
> changes that make -m format configurable.

Great, thank you!

> Alternatively, we can add a configuration option, or let -m imply -p
> only when -m format is explicitly configured by the user.

Since the goal here is simple, easily understandable, and
user-friendly behavior, I think -m should imply -p all the time, or at
least imply -p by default. The less I have to explain to new Git
users, the better.

-Alex

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-29 15:25     ` Alex Henrie
@ 2021-04-29 16:35       ` Sergey Organov
  2021-04-29 17:24         ` Alex Henrie
  2021-05-04 20:09       ` Felipe Contreras
  1 sibling, 1 reply; 28+ messages in thread
From: Sergey Organov @ 2021-04-29 16:35 UTC (permalink / raw)
  To: Alex Henrie; +Cc: Junio C Hamano, Git mailing list

Alex Henrie <alexhenrie24@gmail.com> writes:

> On Wed, Apr 28, 2021 at 9:22 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Alex Henrie <alexhenrie24@gmail.com> writes:
>>
>> > Why do -c and -cc imply -p, but -m does not? I tried to use both `git
>> > log -c` and `git log -m` today and was confused when the latter didn't
>> > produce any output. Could we change this behavior in a future version
>> > of Git?
>>
>> "[alias] lm = log -m" can be used when you only want the logs
>>
>>     $ git lm maint..master
>>
>> or when you want to also view patches your preference is to see all
>> sides of diffs of merges
>>
>>     $ git lm -p maint..mater
>>
>> but depending on who you are that may be of dubious utility.
>>
>> It is best to move on, writing it off as historical accident, and
>> embrace the new --diff-merges=m option, instead of wasting time on
>> pondering "why", because accidents do not have to have a deep reason
>> behind them ;-)
>
> If the behavior is an idiosyncratic accident of dubious utility, let's
> replace it with something that makes sense and is useful :-) If we
> make -m imply -p then no alias is necessary, `git log` would display
> the log without diffs and `git log -m` would display the log with all
> the diffs.
>
> On Thu, Apr 29, 2021 at 6:38 AM Sergey Organov <sorganov@gmail.com> wrote:
>>
>> As the final purpose of all this is to have -m as user-friendly short
>> option, I'd incline to finally let it imply -p, as --diff-merges=m now
>> covers another side of the coin.
>>
>> What do you think?
>
> I am 100% in favor of that proposal, and I can work on the code this
> weekend.

No need to bother. If we agree, I'll send a patch atop of my recent
changes that make -m format configurable.

Alternatively, we can add a configuration option, or let -m imply -p
only when -m format is explicitly configured by the user.

-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-29 12:38   ` Sergey Organov
@ 2021-04-29 15:25     ` Alex Henrie
  2021-04-29 16:35       ` Sergey Organov
  2021-05-04 20:09       ` Felipe Contreras
  2021-04-29 23:27     ` Junio C Hamano
  1 sibling, 2 replies; 28+ messages in thread
From: Alex Henrie @ 2021-04-29 15:25 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Junio C Hamano, Git mailing list

On Wed, Apr 28, 2021 at 9:22 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Alex Henrie <alexhenrie24@gmail.com> writes:
>
> > Why do -c and -cc imply -p, but -m does not? I tried to use both `git
> > log -c` and `git log -m` today and was confused when the latter didn't
> > produce any output. Could we change this behavior in a future version
> > of Git?
>
> "[alias] lm = log -m" can be used when you only want the logs
>
>     $ git lm maint..master
>
> or when you want to also view patches your preference is to see all
> sides of diffs of merges
>
>     $ git lm -p maint..mater
>
> but depending on who you are that may be of dubious utility.
>
> It is best to move on, writing it off as historical accident, and
> embrace the new --diff-merges=m option, instead of wasting time on
> pondering "why", because accidents do not have to have a deep reason
> behind them ;-)

If the behavior is an idiosyncratic accident of dubious utility, let's
replace it with something that makes sense and is useful :-) If we
make -m imply -p then no alias is necessary, `git log` would display
the log without diffs and `git log -m` would display the log with all
the diffs.

On Thu, Apr 29, 2021 at 6:38 AM Sergey Organov <sorganov@gmail.com> wrote:
>
> As the final purpose of all this is to have -m as user-friendly short
> option, I'd incline to finally let it imply -p, as --diff-merges=m now
> covers another side of the coin.
>
> What do you think?

I am 100% in favor of that proposal, and I can work on the code this weekend.

-Alex

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-29  3:22 ` Junio C Hamano
@ 2021-04-29 12:38   ` Sergey Organov
  2021-04-29 15:25     ` Alex Henrie
  2021-04-29 23:27     ` Junio C Hamano
  0 siblings, 2 replies; 28+ messages in thread
From: Sergey Organov @ 2021-04-29 12:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Henrie, Git mailing list

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

> Alex Henrie <alexhenrie24@gmail.com> writes:
>
>> Why do -c and -cc imply -p, but -m does not? I tried to use both `git
>> log -c` and `git log -m` today and was confused when the latter didn't
>> produce any output. Could we change this behavior in a future version
>> of Git?
>
> "[alias] lm = log -m" can be used when you only want the logs
>
>     $ git lm maint..master
>
> or when you want to also view patches your preference is to see all
> sides of diffs of merges
>
>     $ git lm -p maint..mater
>
> but depending on who you are that may be of dubious utility.
>
> It is best to move on, writing it off as historical accident, and
> embrace the new --diff-merges=m option, instead of wasting time on
> pondering "why", because accidents do not have to have a deep reason
> behind them ;-)

Yep, but --diff-merges=m doesn't imply -p either, though it does produce
diff output without -p, for merge commits only.

As the final purpose of all this is to have -m as user-friendly short
option, I'd incline to finally let it imply -p, as --diff-merges=m now
covers another side of the coin.

What do you think?

-- Sergey Organov

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: Why doesn't `git log -m` imply `-p`?
  2021-04-29  1:44 Alex Henrie
@ 2021-04-29  3:22 ` Junio C Hamano
  2021-04-29 12:38   ` Sergey Organov
  2021-05-10 12:11 ` Sergey Organov
  1 sibling, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2021-04-29  3:22 UTC (permalink / raw)
  To: Alex Henrie; +Cc: Git mailing list, Sergey Organov

Alex Henrie <alexhenrie24@gmail.com> writes:

> Why do -c and -cc imply -p, but -m does not? I tried to use both `git
> log -c` and `git log -m` today and was confused when the latter didn't
> produce any output. Could we change this behavior in a future version
> of Git?

"[alias] lm = log -m" can be used when you only want the logs

    $ git lm maint..master

or when you want to also view patches your preference is to see all
sides of diffs of merges

    $ git lm -p maint..mater

but depending on who you are that may be of dubious utility.

It is best to move on, writing it off as historical accident, and
embrace the new --diff-merges=m option, instead of wasting time on
pondering "why", because accidents do not have to have a deep reason
behind them ;-)

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Why doesn't `git log -m` imply `-p`?
@ 2021-04-29  1:44 Alex Henrie
  2021-04-29  3:22 ` Junio C Hamano
  2021-05-10 12:11 ` Sergey Organov
  0 siblings, 2 replies; 28+ messages in thread
From: Alex Henrie @ 2021-04-29  1:44 UTC (permalink / raw)
  To: Git mailing list, Sergey Organov, Junio C Hamano

I read the following in `man git-log` today:

--diff-merges=separate, --diff-merges=m, -m
    This makes merge commits show the full diff with respect to each of
    the parents. Separate log entry and diff is generated for each
    parent.  -m doesn't produce any output without -p.

--diff-merges=combined, --diff-merges=c, -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.  -c implies -p.

--diff-merges=dense-combined, --diff-merges=cc, --cc
    With this option the output produced by --diff-merges=combined is
    further compressed by omitting uninteresting hunks whose contents
    in the parents have only two variants and the merge result picks one
    of them without modification.  --cc implies -p.

Why do -c and -cc imply -p, but -m does not? I tried to use both `git
log -c` and `git log -m` today and was confused when the latter didn't
produce any output. Could we change this behavior in a future version
of Git?

-Alex

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2021-05-19 17:03 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CANvKV6anT0MV7GDeY_Cd2r+BwvMjsTdmjK+s_DKth7ZqL0XRUQ@mail.gmail.com>
2021-05-19 17:03 ` Why doesn't `git log -m` imply `-p`? Sergey Organov
2021-04-29  1:44 Alex Henrie
2021-04-29  3:22 ` Junio C Hamano
2021-04-29 12:38   ` Sergey Organov
2021-04-29 15:25     ` Alex Henrie
2021-04-29 16:35       ` Sergey Organov
2021-04-29 17:24         ` Alex Henrie
2021-04-29 19:07           ` Sergey Organov
2021-05-04 20:09       ` Felipe Contreras
2021-05-04 20:34         ` Sergey Organov
2021-04-29 23:27     ` Junio C Hamano
2021-04-30  4:50       ` Junio C Hamano
2021-04-30 14:00         ` Sergey Organov
2021-05-01  0:41           ` Junio C Hamano
2021-05-03 17:42             ` Sergey Organov
2021-05-04  1:15               ` Junio C Hamano
2021-05-04  9:10                 ` Sergey Organov
2021-05-04 12:38                   ` Junio C Hamano
2021-05-04 14:18                     ` Sergey Organov
2021-05-05  0:20                       ` Junio C Hamano
2021-05-05 13:43                         ` Sergey Organov
2021-05-06  0:27                           ` Junio C Hamano
2021-05-06 12:59                             ` Sergey Organov
2021-05-06 20:29                               ` Junio C Hamano
2021-05-06 20:48                                 ` Sergey Organov
2021-05-07  1:31                                   ` Alex Henrie
2021-05-10 12:11 ` Sergey Organov
2021-05-10 16:56   ` Alex Henrie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).