All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael J Gruber <git@drmicha.warpmail.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC] Should "log --cc" imply "log --cc -p"?
Date: Mon, 04 Feb 2013 12:04:55 +0100	[thread overview]
Message-ID: <510F95D7.6010107@drmicha.warpmail.net> (raw)
In-Reply-To: <7vmwvl6lj9.fsf@alter.siamese.dyndns.org>

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

  reply	other threads:[~2013-02-04 11:05 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-03 23:10 [RFC] Should "log --cc" imply "log --cc -p"? Junio C Hamano
2013-02-04 11:04 ` Michael J Gruber [this message]
2013-02-04 16:36   ` Junio C Hamano
2013-02-05  8:58     ` Michael J Gruber
2013-02-05  9:33     ` Jeff King
2013-02-05 15:46       ` Junio C Hamano
2013-02-05 10:16     ` Ævar Arnfjörð Bjarmason
2013-02-05 11:22       ` Jeff King
2013-02-05 14:27         ` Ævar Arnfjörð Bjarmason
2013-02-05 15:51           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=510F95D7.6010107@drmicha.warpmail.net \
    --to=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.