From: Damien Diederen <dd@crosstwine•com>
To: git@vger•kernel.org
Cc: Paul Mackerras <paulus@samba•org>
Subject: [RFC BANDAID PATCH] gitk: Don't add missing parents to display list when pickaxing
Date: Wed, 2 Jun 2010 22:32:40 +0200 [thread overview]
Message-ID: <87ljaxcffb.fsf@mini.crosstwine.com> (raw)
Gitk unconditionally prepends --parents --boundary to the
user-provided argument list, and relies on boundary commits to
"neatly" close history lines.
Git log's '-S' option, however, interacts badly with --boundary as it
is implemented as a post-processing filter of the object graph.
Results coming from a common sub-graph end up producing no boundary,
yet may appear disjoint as no parent rewriting occurs. In git.git:
$ git log -SGMail --oneline --parents --boundary
e498257 bd7440f Documentation/SubmittingPatches: clarify GMail section and SMTP
df5753c a751b5c SubmittingPatches: update GMail section
50dffd4 28001d0 Google has renamed the imap folder
This confuses gitk, which truncates its "display order"
list (containing the six commits he knows about--three plus three
parents) to a three-row display:
@ Documentation/SubmittingPatches: clarify GMail section and SMTP
|
O show-branch: use DEFAULT_ABBREV instead of 7
@ SubmittingPatches: update GMail section
Note how it picked up "show-branch: ..." (bd7440f) as a
pseudo-boundary of e498257, marking it with an "empty" bullet, but
then stopped prematurely--missing one of the matching commits!
This bandaid patch cause gitk not to try to "close" the arcs when
pickaxe is in effect. "Missing parents" are not added to the display
order list, whose length then matches the number of rows. It produces
the following imperfect display, which has the advantage of not
omitting any result:
@ Documentation/SubmittingPatches: clarify GMail section and SMTP
|
| @ SubmittingPatches: update GMail section
| |
v v @ Google has renamed the imap folder
The parallel history lines are incorrect, and the arrows don't point
anywhere. Fixing this would require more extensive changes to gitk's
display engine, and possibly invalidate some of the invariants on
which it currently relies.
Preliminary investigation suggests that adding "proper" --boundary
support to pickaxe would require to:
1. disable rev_info->boundary when producing the revision list;
2. manually maintain and trim rev_info->boundary_commits in
cmd_log_walk according to the return value of log_tree_commit;
3. perform a second pass of 'log_tree_commit' on the "fake" boundary,
with pickaxe filtering disabled.
This seems to be a superior solution, but is not too pretty, and I may
be missing something (besides the fact that it slightly changes the
definition of boundary). It may also fix --graph's current behaviour,
which is, err..., interesting.
I have not looked into parent rewriting, which may be the correct
thing to do. Note, however, that it seems it would not match the way
e.g. 'gitk --grep=GMail' works--at a first glance.
Comments? Suggestions?
---
gitk | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/gitk b/gitk
index 45e3380..5cd3787 100755
--- a/gitk
+++ b/gitk
@@ -130,7 +130,7 @@ proc unmerged_files {files} {
}
proc parseviewargs {n arglist} {
- global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env
+ global vdatemode vmergeonly vflags vdflags vrevs vfiltered vnoboundary vorigargs env
set vdatemode($n) 0
set vmergeonly($n) 0
@@ -141,6 +141,7 @@ proc parseviewargs {n arglist} {
set origargs $arglist
set allknown 1
set filtered 0
+ set noboundary 0
set i -1
foreach arg $arglist {
incr i
@@ -194,6 +195,9 @@ proc parseviewargs {n arglist} {
# These mean that we get a subset of the commits
set filtered 1
lappend glflags $arg
+ if {[string match "-S*" $arg]} {
+ set noboundary 1
+ }
}
"-n" {
# This appears to be the only one that has a value as a
@@ -237,6 +241,7 @@ proc parseviewargs {n arglist} {
set vflags($n) $glflags
set vrevs($n) $revargs
set vfiltered($n) $filtered
+ set vnoboundary($n) $noboundary
set vorigargs($n) $origargs
return $allknown
}
@@ -1360,6 +1365,7 @@ proc getcommitlines {fd inst view updating} {
global parents children curview hlview
global idpending ordertok
global varccommits varcid varctok vtokmod vfilelimit
+ global vnoboundary
set stuff [read $fd 500000]
# git log doesn't terminate the last commit with a null...
@@ -1401,7 +1407,9 @@ proc getcommitlines {fd inst view updating} {
set viewcomplete($view) 1
# Check if we have seen any ids listed as parents that haven't
# appeared in the list
- closevarcs $view
+ if {!$vnoboundary($view)} {
+ closevarcs $view
+ }
notbusy $view
}
if {$view == $curview} {
--
1.7.1
reply other threads:[~2010-06-02 21:06 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=87ljaxcffb.fsf@mini.crosstwine.com \
--to=dd@crosstwine$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=paulus@samba$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox