Re: [PATCH 4/4] builtin/show: do not prune by pathspec

[Junio C Hamano <gitster@pobox.com>]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> diff --git a/builtin/log.c b/builtin/log.c
> index 916019c..474a76d 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -420,6 +420,8 @@ int cmd_show(int argc, const char **argv, const char *prefix)
>  	opt.def = "HEAD";
>  	opt.tweak = show_rev_tweak_rev;
>  	cmd_log_init(argc, argv, prefix, &rev, &opt);
> +	if (rev.no_walk)
> +		rev.prune = 0;

This is not your fault, but I am somewhat disgusted by the reason why

    $ git show master..next [ -- Documentation ]

works by "walking" the history.  It takes completely different codepath
from "git log" with the same set of arguments.

 * first log_init() grabs '^master' and 'next' into the rev.pending object
   array;

 * we pop '^master' first, make it the only object in the rev.pending
   object array, and let cmd_log_walk() call prepare_revision_walk() on it
   to limit the list.  Since we don't have any positive ref, we get
   nothing;

 * then we pop 'next', make it again _the only_ object in the rev.pending
   object array; prepare_revision_walk() on the same codepath now limits
   the list to exclude what is reachable from 'master', only because we
   have processed '^master'.

Yikes.  In other words, the reason the current code works is only by
accident.

This is a tangent, but we _might_ want to (perhaps at git 2.0) update the
revision parsing machinery so that we can clear the object flags more
easily and have

    $ git log A..B C..D [ -- pathspec ]

compute the union of (commits reachable from B but not from A) and
(commits reachable from D but not from C).  I say we _might_ because we
would still want to compute what the current code computes in some cases,
and we may be able to express it as "^A ^C B D", but I am not sure if we
want to go that route and end up with a general set operation (with unions
and intersections, perhaps even using parentheses to express precedence).

More general set operation is certainly doable, and at that point it
probably does not matter that we cannot stream the output as we are doing
something complex (IOW, we would be revs->limited, giving up the latency),
but I don't know if it is useful in the first place to support such
general case.  Because the most complex set operations that I run every
day is

    $ git log ^master $topic1 $topic2 $topic3... -- $pathspec

and I don't recall cases in which I wished A..B C..D showed commits
within A..B that are reachable from C.