Re: [PATCH] [GSoC][MICRO] Forbid "log --graph --no-walk"

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 6, 2015, 18:51 UTC
Message-ID: <xmqqioeeyn6q.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CABwkPcoBG61YY0N4b33gdMq2cC1oQ_wFV7KaGGu3wYfuY9inGw@mail.gmail.com>

Dongcan Jiang <dongcan.jiang@gmail.com> writes:

Show 7 quoted lines

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