From: Junio C Hamano <gitster@pobox.com>
To: Dongcan Jiang <dongcan.jiang@gmail.com>
Cc: "Eric Sunshine" <sunshine@sunshineco.com>,
"René Scharfe" <l.s.r@web.de>, "Git List" <git@vger.kernel.org>
Subject: Re: [PATCH] [GSoC][MICRO] Forbid "log --graph --no-walk"
Date: Fri, 06 Mar 2015 10:51:09 -0800 [thread overview]
Message-ID: <xmqqioeeyn6q.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CABwkPcoBG61YY0N4b33gdMq2cC1oQ_wFV7KaGGu3wYfuY9inGw@mail.gmail.com> (Dongcan Jiang's message of "Fri, 6 Mar 2015 21:13:26 +0800")
Dongcan Jiang <dongcan.jiang@gmail.com> writes:
> At first, I also tried to only judge the value of "revs->no_walk &&
> revs->graph", but unfortunately, it failed to pass all cases in
> t4052-stat-output.sh.
> e.g. command "git show --stat --graph" failed to get the correct result.
>
> Finally, this is because that "revs->no_walk" gets set when it comes
> to "git show --stat".
When "git show" is given a range, it turns no-walk off and becomes a
command about a connected history. Otherwise, it is a command about
discrete point(s).
Because "git show --graph A B C" and "git log --graph --no-walk A B
C" are moral equivalents, if we are forbidding the latter, we should
forbid the former.
"git show A" (and no other revs, just a single point), however,
could be thought of as an equivalent for "git log -1 A", even though
that interpretation is based on a wrong world view (because "show
one and stop" is not the reason why the result "git show A" gives
has only A and nothing else). And it makes sort of sense to allow
"git show --graph A" that is an equivalent to "git log -1 --graph A"
under that interpretation.
I was tempted to say the existing test is wrong to expect that
"--graph" does anything meaningful for "git show" that is run for a
non-range. But doing the "use of both is wrong for log" change
without special casing "show" (and instead "fixing" t4052) would be
a change in behaviour, which we try to avoid in general.
I'd prefer a solution that does not waste a new bit in revision
structure in general, but if we were to waste one bit to special
case "show", it shouldn't be "fail only for the log command". It
should be "allow this stupidity only for the show command for
backward compatibility" bit instead, I think.
next prev parent reply other threads:[~2015-03-06 18:51 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-06 8:55 [PATCH] [GSoC][MICRO] Forbid "log --graph --no-walk" Dongcan Jiang
2015-03-06 9:56 ` Eric Sunshine
2015-03-06 13:13 ` Dongcan Jiang
2015-03-06 18:51 ` Junio C Hamano [this message]
2015-03-06 10:07 ` René Scharfe
2015-03-07 4:56 ` [PATCH v2/GSoC/MICRO] revision: forbid combining --graph and --no-walk Dongcan Jiang
2015-03-08 19:40 ` Junio C Hamano
2015-03-09 4:09 ` [PATCH v3/GSoC/MICRO] " Dongcan Jiang
2015-03-10 21:39 ` Junio C Hamano
2015-03-11 2:13 ` [PATCH v4/GSoC/MICRO] " Dongcan Jiang
2015-03-17 23:18 ` Junio C Hamano
2015-03-17 23:24 ` Eric Sunshine
2015-03-18 5:34 ` [PATCH v5/GSoC/MICRO] " Dongcan Jiang
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=xmqqioeeyn6q.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=dongcan.jiang@gmail.com \
--cc=git@vger.kernel.org \
--cc=l.s.r@web.de \
--cc=sunshine@sunshineco.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.