From: Michael J Gruber <git@drmicha.warpmail.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Johannes Sixt" <j6t@kdbg.org>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
"Linus Torvalds" <torvalds@linux-foundation.org>
Subject: Re: [PATCH 4/4] builtin/show: do not prune by pathspec
Date: Tue, 05 Apr 2011 08:06:51 +0200 [thread overview]
Message-ID: <4D9AB17B.70409@drmicha.warpmail.net> (raw)
In-Reply-To: <7v1v1h8z5k.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 04.04.2011 23:49:
> 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.
That is super ugly, yes.
>
> 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.
>
We seem to promise that one should think in terms of sets, and this
breaks with "A..B" and "C...D" unless one always thinks of them in the
resolved form "^A B" and "^E C D" (with E=$(git merge-base C D)).
I also think that "A...B C" treats C as the right of that symmetric
range (because anything non-left we treat as right), and "A...B ^C"
probably interacts badly with --cherry-pick (though I haven't tried).
Also, "A...B C...D" would be a candidate for union. Lot's to do for
after 1.7.5...
Michael
next prev parent reply other threads:[~2011-04-05 6:10 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-31 6:45 Usability improvement request: git show revision -- file Piotr Krukowiecki
2011-03-31 7:50 ` Michael J Gruber
2011-03-31 9:17 ` [PATCH 1/3] t1506: factor out test for "Did you mean..." Michael J Gruber
2011-03-31 9:17 ` [PATCH 2/3] sha1_name: Suggest commit:./file for path in subdir Michael J Gruber
2011-03-31 19:26 ` Junio C Hamano
2011-04-01 6:52 ` Michael J Gruber
2011-04-01 19:11 ` Junio C Hamano
2011-03-31 9:17 ` [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec Michael J Gruber
2011-03-31 10:18 ` Johannes Sixt
2011-03-31 10:58 ` Michael J Gruber
2011-03-31 11:42 ` Johannes Sixt
2011-03-31 12:07 ` Michael J Gruber
2011-03-31 12:50 ` Nguyen Thai Ngoc Duy
2011-03-31 13:26 ` Michael J Gruber
2011-03-31 13:35 ` Nguyen Thai Ngoc Duy
2011-03-31 13:55 ` Michael J Gruber
2011-03-31 19:23 ` Junio C Hamano
2011-04-01 6:46 ` Michael J Gruber
2011-04-01 9:20 ` [PATCH 0/4] reflog, show and command line overrides Michael J Gruber
2011-04-01 9:20 ` [PATCH 1/4] builtin/log.c: separate default and setup of cmd_log_init() Michael J Gruber
2011-04-01 9:20 ` [PATCH 2/4] t/t1411: test reflog with formats Michael J Gruber
2011-04-01 9:20 ` [PATCH 3/4] reflog: fix overriding of command line options Michael J Gruber
2011-04-01 9:20 ` [PATCH 4/4] builtin/show: do not prune by pathspec Michael J Gruber
2011-04-01 21:50 ` Junio C Hamano
2011-04-01 22:59 ` Junio C Hamano
2011-04-03 13:16 ` Michael J Gruber
2011-04-04 21:49 ` Junio C Hamano
2011-04-05 6:06 ` Michael J Gruber [this message]
2011-03-31 19:59 ` Usability improvement request: git show revision -- file Piotr Krukowiecki
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D9AB17B.70409@drmicha.warpmail.net \
--to=git@drmicha.warpmail.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=pclouds@gmail.com \
--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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).