Re: [RFC] Should "log --cc" imply "log --cc -p"?

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Feb 4, 2013, 11:04 UTC
Message-ID: <510F95D7.6010107@drmicha.warpmail.net>
In-Reply-To: <7vmwvl6lj9.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 04.02.2013 00:10:

Show 10 quoted lines

> I think a natural way to ask reviewing the recent merges while
> showing tricky ones would be to say:
> 
> 	$ git log --first-parent --cc master..pu
> 
> But this does not to show what I expect to see, which is an output
> of:
> 
> 	$ git log --first-parent --cc -p master..pu
>

I'm not Junio, but I guess he would respond that it's a matter of expectations: "--cc" is a diff option, and just like any other diff option, it has an effect on "log" only if "log" has been told to show a diff.

Oh wait, you *are* Junio ;)

> This is only a minor irritation, but I think it might make sense to
> make it notice the --cc in the former and turn -p on automatically.

I think we have a clear distiction between rev-list/log options and diff options. "log" comes without a diff, "-p" turns on diffs.

"log" passes diff-options to "diff". If the user gives a diff-option to "log" we can conclude that he meant to request a diff and turn them on automatically (as if "-p" was there also).

But I'm wondering whether that has adverse effects on scripts/aliases. For example, I could have a special alias

newpu = log first-parent --cc next..pu

which allows me to use "git newpu" as well as "git newpu -p" to get those new commits with or without diff in my preferred format, but not any more after the change you suggest. (I could use a second alias, of course; and yes, I'm (ab)using current option parser features.)

Show 17 quoted lines

> The same for
> 
> 	$ git log --cc next~3..next
> 
> which may make sense to turn into "git log -p --cc next~3..next".
> 
> When deciding if the above makes sense, there are a few things to
> know to be true as prerequisites for the discussion:
> 
>  * Neither of these
> 
> 	$ git log --first-parent -p master..pu
> 	$ git log -p master..pu
> 
>    shows any patches, and it is not a bug.  No patches are shown for
>    merges unless -m is given, and when -m is given, we give pairwise
>    2-way diffs for the number of parents.

But diffs are on here ("-p"), it's just that the default diff option for merges is to not display them. Well, I admit there's two different ways of thinking here:

A) "git log -p" turns on diffs for all commits, and the default diff options is the (non-existing) "--no-show" diff-option for merges.

B) "git log -p" applies "-p" to all commits except merge commits.

I'm strongly in the A camp, because I think that this gives a clearer interface. A really describes the user facing side, whereas B is closer to the implementation.

Show 6 quoted lines

>  * We recently tweaked this:
> 
> 	$ git log --first-parent -m -p master..pu
> 
>    to omit diffs with second and later parents, as that is what the
>    user wishes with --first-parent.

That made --first-parent into a dual-purpose option, i.e. it modifies the rev-listing to one parent as well as the diff creation. It does not turn on diffs by itself.

Show 7 quoted lines

>  * The "--cc" option, when comparing two trees (i.e. showing a
>    non-merge commit), is designed to show a normal patch.  In other
>    words, you can view "--cc" as a modifier when you request a patch
>    output format with "-p".  For "git show", "--cc -p" is turned on
>    by default, and giving "-m" explicity (i.e. "git show -m") you
>    can turn it off and have it do "-m -p" instead.
>

Yes, I really think of --cc is a diff-option, and that this point of view gives the clearest ui.

We could argue that any diff-option could switch on diffs (imply -p), but that change seems to be quite radical. "show" having "-p" as a default is quite natural, but that is different.

Having only "--cc" imply "-p" would be too much special case auto-magic for my taste. We have too many of these already.

I really think we should leave it as is.

Michael