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

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

 



Junio C Hamano venit, vidit, dixit 04.02.2013 00:10:
> 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.)

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

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

>  * 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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]