git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Carlos Martín Nieto" <cmn@elego.de>
Cc: Gustaf Hendeby <hendeby@isy.liu.se>,
	git@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: git show and the --quiet option
Date: Sat, 28 May 2011 11:27:25 -0700	[thread overview]
Message-ID: <7vtycepto2.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7vy61qpv59.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Sat, 28 May 2011 10:55:30 -0700")

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

>> I was playing around with "git show" lately and realized it has changed
>> its behavior regarding the --quiet option, which no longer suppresses
>> the diff output as it used to.
>
> The official and right way to suppress diff output from "show" has always
> been with the "-s" option, and it should still work. Otherwise please
> report a bug here.

Having said that, I think this is a minor regression.

"git show A B C" has been about showing objects (not commits) A and B and
C. It picks a convenient format for human consumption for each type, and
shows commit as if it were given to "log -1 -p", tree as if it were given
to "ls-tree --name-only", blob as if it were given to "cat-file blob".

But recently, Linus bolted "git show A..B" on to it, and in a way that is
quite wrong. It walks the history by accident, not by design. This makes
fixing this regression somewhat complex, I suspect.

Given the existing machinery to "show" each individual object given from
the command line, one would naturally imagine that we have a routine that
takes one object, tells its object type, and formats the object in the
representation suitable for human consumption, and have a loop over the
command line to call that routine with each object from the command line.
And "git show A..B" would first walk to find individual commit objects
between A and B, and feed the same routine with these commits one by one.

Not so. The current implementation walks the history between A..B as a
side effect of showing a commit (starting from B) and works by pure
accident.

Case in point: "git show master master" shows two copies of the same
commit, as it should. "git show master^..master master" does not. The
reason? Walking between "master^..master" is done as a side effect of
showing "master^..master" and marks commit object "master" already shown,
and makes the command ignore the second argument.

A worse example can be seen by running something silly like "git show
master~10 master^..master", which you would expect to see two commits
(master~10 and master).  Do not do this on anything with a deep history
like the kernel repository---it will walk down to the root commit.

I think the ideal fix would be to fix the "show A..B" support (one
possible solution would be to simply disable it, but I'd see it as the
last resort) so that it first collects the commit objects in a queue by
properly walking (and clean the object flags that were used to control the
walking after we know what commits are in the range), expand A..B into
these commits on the command line arguments list, and then run the
resulting command line arguments through the traditional "git show"
machinery that shows one object at a time.

If we go that route, then we should always use "quiet" during the internal
history walking that expands A..B to the set of commits in the range with
or without command line --quiet. And then make both --quiet and -s from
the command line to control if the patch is shown when showing a commit.

      reply	other threads:[~2011-05-28 18:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-28 16:53 git show and the --quiet option Gustaf Hendeby
2011-05-28 17:26 ` Carlos Martín Nieto
2011-05-28 18:03   ` Gustaf Hendeby
2011-05-29 13:24     ` Carlos Martín Nieto
2011-05-28 19:17   ` Junio C Hamano
2011-05-30  9:32     ` Carlos Martín Nieto
2011-06-02 18:26       ` Drew Northup
2011-06-05 23:13         ` Carlos Martín Nieto
2011-05-28 17:55 ` Junio C Hamano
2011-05-28 18:27   ` Junio C Hamano [this message]

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=7vtycepto2.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=cmn@elego.de \
    --cc=git@vger.kernel.org \
    --cc=hendeby@isy.liu.se \
    --cc=torvalds@linux-foundation.org \
    /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 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).