* [BUG] gitk: breaks with both version and file limits
@ 2006-04-16 11:54 Yann Dirson
2006-04-16 21:51 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Yann Dirson @ 2006-04-16 11:54 UTC (permalink / raw)
To: Paul Mackerras, GIT list
As an example, on a current git tree, the following command triggers
an 'Error: expected boolean value but got ""' dialog when scrolling to
the bottom of the graph, and leaves the bottom of the graph not drawn.
This happens with current master, but not with 1.2.4.
gitk --all ^v1.0.6 -- Makefile
While checking with git-bisect while limiting to path gitk, I noticed
the amusing detail that "git-bisect visualize" is impacted by this bug.
git-bisect shows that the commit triggering the problem is
79b2c75e043ad85f9a6b1a8d890b601a2f761a0e ("gitk: replace parent and
children arrays with lists" on Apr 2)
Hope this helps,
--
Yann Dirson <ydirson@altern.org> |
Debian-related: <dirson@debian.org> | Support Debian GNU/Linux:
| Freedom, Power, Stability, Gratis
http://ydirson.free.fr/ | Check <http://www.debian.org/>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [BUG] gitk: breaks with both version and file limits 2006-04-16 11:54 [BUG] gitk: breaks with both version and file limits Yann Dirson @ 2006-04-16 21:51 ` Junio C Hamano 2006-04-17 0:38 ` Paul Mackerras 2006-04-17 1:53 ` [PATCH] rev-list --boundary: show boundary commits even when limited otherwise Junio C Hamano 0 siblings, 2 replies; 4+ messages in thread From: Junio C Hamano @ 2006-04-16 21:51 UTC (permalink / raw) To: Yann Dirson; +Cc: git, Paul Mackerras Yann Dirson <ydirson@altern.org> writes: > As an example, on a current git tree, the following command triggers > an 'Error: expected boolean value but got ""' dialog when scrolling to > the bottom of the graph, and leaves the bottom of the graph not drawn. > This happens with current master, but not with 1.2.4. A workaround errorproofing would be to do something like this (note that I do not really speak Tcl). The real issue is "git-rev-list --boundary ran..ge -- path" does not show boundary. I am not sure if it even worked in the past; will take a look. -- >8 -- gitk: [lindex $list $no_such_idx] returns "" which is not a valid bool When asking for commit that is not listed for some reason in commitlisted, gitk barfed because return value from lindex was "" which was not a bool acceptable in ($bool ? A : B) construct. We are expecting "1" here for listed commits (this comes from getcommitlines), so explicitly check for it. Signed-off-by: Junio C Hamano <junkio@cox.net> --- diff --git a/gitk b/gitk index f88c06e..5d2b0cc 100755 --- a/gitk +++ b/gitk @@ -1460,7 +1460,7 @@ proc drawcmittext {id row col rmx} { global linehtag linentag linedtag global mainfont namefont canvxmax - set ofill [expr {[lindex $commitlisted $row]? "blue": "white"}] + set ofill [expr {("1" == [lindex $commitlisted $row]) ? "blue" : "white"}] set x [xc $row $col] set y [yc $row] set orad [expr {$linespc / 3}] ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [BUG] gitk: breaks with both version and file limits 2006-04-16 21:51 ` Junio C Hamano @ 2006-04-17 0:38 ` Paul Mackerras 2006-04-17 1:53 ` [PATCH] rev-list --boundary: show boundary commits even when limited otherwise Junio C Hamano 1 sibling, 0 replies; 4+ messages in thread From: Paul Mackerras @ 2006-04-17 0:38 UTC (permalink / raw) To: Junio C Hamano; +Cc: Yann Dirson, git Junio C Hamano writes: > The real issue is "git-rev-list --boundary ran..ge -- path" does > not show boundary. I am not sure if it even worked in the past; > will take a look. Ah... and in addextraid I wasn't appending a 0 to commitlisted, so commitlisted got out of sync with the displayorder list. I just committed this patch, which keeps them in sync. I think that's preferable to working around the missing elements. Paul. diff --git a/gitk b/gitk index f88c06e..87e7162 100755 --- a/gitk +++ b/gitk @@ -1116,11 +1116,12 @@ proc layoutrows {row endrow last} { proc addextraid {id row} { global displayorder commitrow commitinfo - global commitidx + global commitidx commitlisted global parentlist childlist children incr commitidx lappend displayorder $id + lappend commitlisted 0 lappend parentlist {} set commitrow($id) $row readcommit $id @@ -1500,7 +1501,7 @@ proc drawcmittext {id row col rmx} { proc drawcmitrow {row} { global displayorder rowidlist global idrowranges idrangedrawn iddrawn - global commitinfo commitlisted parentlist numcommits + global commitinfo parentlist numcommits if {$row >= $numcommits} return foreach id [lindex $rowidlist $row] { ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] rev-list --boundary: show boundary commits even when limited otherwise. 2006-04-16 21:51 ` Junio C Hamano 2006-04-17 0:38 ` Paul Mackerras @ 2006-04-17 1:53 ` Junio C Hamano 1 sibling, 0 replies; 4+ messages in thread From: Junio C Hamano @ 2006-04-17 1:53 UTC (permalink / raw) To: Yann Dirson; +Cc: git, Linus Torvalds The boundary commits are shown for UI like gitk to draw them as soon as topo-order sorting allows, and should not be omitted by get_revision() filtering logic. As long as their immediate child commits are shown, we should not filter them out. Signed-off-by: Junio C Hamano <junkio@cox.net> --- > The real issue is "git-rev-list --boundary ran..ge -- path" does > not show boundary. I am not sure if it even worked in the past; > will take a look. I _think_ this fixes it, but this is rev-list which I feel a bit uneasy touching. A lower-impact approach of not filtering BOUNDARY commits at all does not work well; it shows many isolated open-circle commits in gitk that do not touch the specified paths. I got tired of updating the private object.flags users, so just reserved the lower 16 bits for use of revision.c while I was at it. http-push.c | 10 +++++----- rev-list.c | 4 ++-- revision.c | 29 +++++++++++++++++++++++++++-- revision.h | 3 ++- 4 files changed, 36 insertions(+), 10 deletions(-) 298196646f35b6f5bc2131074204826d39aff8cf diff --git a/http-push.c b/http-push.c index 19a0f77..e7f7f44 100644 --- a/http-push.c +++ b/http-push.c @@ -60,12 +60,12 @@ #define LOCK_REQUEST "<?xml version=\"1. #define LOCK_TIME 600 #define LOCK_REFRESH 30 -/* bits #0-6 in revision.h */ +/* bits #0-15 in revision.h */ -#define LOCAL (1u << 7) -#define REMOTE (1u << 8) -#define FETCHING (1u << 9) -#define PUSHING (1u << 10) +#define LOCAL (1u<<16) +#define REMOTE (1u<<17) +#define FETCHING (1u<<18) +#define PUSHING (1u<<19) /* We allow "recursive" symbolic refs. Only within reason, though */ #define MAXDEPTH 5 diff --git a/rev-list.c b/rev-list.c index 963707a..f5511e7 100644 --- a/rev-list.c +++ b/rev-list.c @@ -8,9 +8,9 @@ #include "tree-walk.h" #include "diff.h" #include "revision.h" -/* bits #0-6 in revision.h */ +/* bits #0-15 in revision.h */ -#define COUNTED (1u<<7) +#define COUNTED (1u<<16) static const char rev_list_usage[] = "git-rev-list [OPTION] <commit-id>... [ -- paths... ]\n" diff --git a/revision.c b/revision.c index 0505f3f..e1f9816 100644 --- a/revision.c +++ b/revision.c @@ -750,6 +750,17 @@ static void rewrite_parents(struct rev_i } } +static void mark_boundary_to_show(struct commit *commit) +{ + struct commit_list *p = commit->parents; + while (p) { + commit = p->item; + p = p->next; + if (commit->object.flags & BOUNDARY) + commit->object.flags |= BOUNDARY_SHOW; + } +} + struct commit *get_revision(struct rev_info *revs) { struct commit_list *list = revs->commits; @@ -787,8 +798,20 @@ struct commit *get_revision(struct rev_i } if (commit->object.flags & SHOWN) continue; - if (!(commit->object.flags & BOUNDARY) && - (commit->object.flags & UNINTERESTING)) + + /* We want to show boundary commits only when their + * children are shown. When path-limiter is in effect, + * rewrite_parents() drops some commits from getting shown, + * and there is no point showing boundary parents that + * are not shown. After rewrite_parents() rewrites the + * parents of a commit that is shown, we mark the boundary + * parents with BOUNDARY_SHOW. + */ + if (commit->object.flags & BOUNDARY_SHOW) { + commit->object.flags |= SHOWN; + return commit; + } + if (commit->object.flags & UNINTERESTING) continue; if (revs->min_age != -1 && (commit->date > revs->min_age)) continue; @@ -801,6 +824,8 @@ struct commit *get_revision(struct rev_i if (revs->parents) rewrite_parents(revs, commit); } + if (revs->boundary) + mark_boundary_to_show(commit); commit->object.flags |= SHOWN; return commit; } while (revs->commits); diff --git a/revision.h b/revision.h index 8970b57..4b27043 100644 --- a/revision.h +++ b/revision.h @@ -7,7 +7,8 @@ #define TREECHANGE (1u<<2) #define SHOWN (1u<<3) #define TMP_MARK (1u<<4) /* for isolated cases; clean after use */ #define BOUNDARY (1u<<5) -#define ADDED (1u<<6) /* Parents already parsed and added? */ +#define BOUNDARY_SHOW (1u<<6) +#define ADDED (1u<<7) /* Parents already parsed and added? */ struct rev_info; -- 1.3.0.rc4.g878b ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2006-04-17 1:54 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-04-16 11:54 [BUG] gitk: breaks with both version and file limits Yann Dirson 2006-04-16 21:51 ` Junio C Hamano 2006-04-17 0:38 ` Paul Mackerras 2006-04-17 1:53 ` [PATCH] rev-list --boundary: show boundary commits even when limited otherwise Junio C Hamano
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 NNTP newsgroup(s).