Re: [WRONG/PATCH 1/3] revisions: clarify handling of --no-walk and --do-walk

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Apr 26, 2011, 08:19 UTC
Message-ID: <4DB67FFF.1060709@drmicha.warpmail.net>
In-Reply-To: <20110425002124.GB31168@elie>

Jonathan Nieder venit, vidit, dixit 25.04.2011 02:21:

Show 26 quoted lines

> Michael J Gruber wrote:
> 
>> We don't do the systematic approach now. In some situations, some
>> commands switch on the walker automatically (I think "show A..B") to
>> make things more useful (to most users) but less systematic, even less
>> predictable if you don't know these deviations/exceptions. I've
>> suggested such a "usefulness exception" myself (don't prune commits by
>> path for "show").
> 
> Ah.  To be clearer about the present state:
> 
>  - cmd_show reimplements much of get_revision, to work around the
>    revision walking machinery's lack of callbacks to print tags,
>    blobs, and so on.
> 
>  - ^A means "--do-walk ^A", and A..B means "--do-walk ^A B".  This
>    holds in rev-list, log, show, etc --- they all share the code that
>    does this.
> 
>  - Similarly, -5 means "--do-walk -5".
> 
>  - rev-parse shares a revision parser (get_sha1) with rev-list, but it
>    doesn't share an option parser (alas).
> 
> I personally kind of like the "don't prune commits by path with
> --no-walk" idea.  Not sure what happened to that.

It got dropped after the switch to the new cycle. The question/problem was whether some people used "git show A B C -- path" as a kind of commit filter, selecting only commits which touch path (i.e. prune commits by path). I would claim it's the wrong command to do that and was never documented anyways...

So, my choice would be "do not prune commits unless --do-walk was given" (where "was given" may include "was switched on by our range=>walk logic").

Another choice would be "do not prune if there is only one rev argument and no --do-walk".

Show 6 quoted lines

> Ah.  To be clearer about the present state:
> 
>  - cmd_show reimplements much of get_revision, to work around the
>    revision walking machinery's lack of callbacks to print tags,
>    blobs, and so on.
>

You mean, other than pretty formats... I think our code does a couple of things by hand which could be done with a pretty format. Though a callback may be more efficient, unless our formats are pre-parsed.

Michael