10 messages, 3 authors, 2016-06-15 · open the first message on its own page
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:22
Paul Mackerras [off-list ref] writes:
Would it be possible to put the '-' in only for the last child that has that parent?
Not trivially. We do not keep track of who are children of a commit.
From: Paul Mackerras <hidden>
Date: 2016-06-15 22:42:22
quoted
Would it be possible to put the '-' in only for the last child that has that parent?Not trivially. We do not keep track of who are children of a commit.
Hmmm... how does the --topo-order logic ensure that parents are shown after all of their children? Essentially I want that logic applied to the boundary parent commits as well as the requested commits. The other thing is that if git-rev-list can actually list those boundary parents, complete with the whole commit message if --header is given, then that will save gitk from having to do a git-cat-file to get that information. Paul.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:22
Paul Mackerras [off-list ref] writes:
quoted
Not trivially. We do not keep track of who are children of a commit.Hmmm... how does the --topo-order logic ensure that parents are shown after all of their children? Essentially I want that logic applied to the boundary parent commits as well as the requested commits.
The sort happens after we sift which commits are interesting and which are not, and uninteresting ones are not subject to sorting, so that is too late.
The other thing is that if git-rev-list can actually list those boundary parents, complete with the whole commit message if --header is given, then that will save gitk from having to do a git-cat-file to get that information.
How about this alternative patch, then? It turned out to be
quite convoluted as I feared.
For example, with this graph:
C side
/
A---B---D master
This command
$ git rev-list --boundary --header --parents side..master
would give:
D B
tree D^{tree}
parent B
...
\0-B A
tree B^{tree}
parent A
...
\0
That is, it includes the UNINTERESING commits at the boundary,
which are usually not shown, in its output, but their object
names are prefixed with a '-'.
---diff --git a/rev-list.c b/rev-list.c
index 441c437..a1f129b 100644
--- a/rev-list.c
+++ b/rev-list.c@@ -7,9 +7,9 @@ #include "diff.h" #include "revision.h" -/* bits #0-4 in revision.h */ +/* bits #0-5 in revision.h */ -#define COUNTED (1u<<5) +#define COUNTED (1u<<6) static const char rev_list_usage[] = "git-rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
@@ -51,6 +51,8 @@ printf("%lu ", commit->date); if (commit_prefix[0]) fputs(commit_prefix, stdout); + if (commit->object.flags & BOUNDARY) + putchar('-'); fputs(sha1_to_hex(commit->object.sha1), stdout); if (show_parents) { struct commit_list *parents = commit->parents;
diff --git a/revision.c b/revision.c
index d67718c..a9b8f9d 100644
--- a/revision.c
+++ b/revision.c@@ -390,6 +390,21 @@ } } +void debug_list(struct commit_list *l) +{ + int i = 0; + while (l) { + struct commit *commit = l->item; + printf("%d: %x %s\n", + i, + commit->object.flags, + sha1_to_hex(commit->object.sha1)); + printf(" %s\n", commit->buffer); + l = l->next; + i++; + } +} + static void limit_list(struct rev_info *revs) { struct commit_list *list = revs->commits;
@@ -418,6 +433,27 @@ if (revs->min_age != -1 && (commit->date > revs->min_age)) continue; p = &commit_list_insert(commit, p)->next; + } + if (revs->boundary) { + list = newlist; + while (list) { + struct commit *commit = list->item; + struct object *obj = &commit->object; + struct commit_list *parent = commit->parents; + if (obj->flags & (UNINTERESTING|BOUNDARY)) { + list = list->next; + continue; + } + while (parent) { + struct commit *pcommit = parent->item; + parent = parent->next; + if (!(pcommit->object.flags & UNINTERESTING)) + continue; + pcommit->object.flags |= BOUNDARY; + p = &commit_list_insert(pcommit, p)->next; + } + list = list->next; + } } revs->commits = newlist; }
@@ -587,10 +623,14 @@ revs->remove_empty_trees = 1; continue; } - if (!strncmp(arg, "--no-merges", 11)) { + if (!strcmp(arg, "--no-merges")) { revs->no_merges = 1; continue; } + if (!strcmp(arg, "--boundary")) { + revs->boundary = 1; + continue; + } if (!strcmp(arg, "--objects")) { revs->tag_objects = 1; revs->tree_objects = 1;
@@ -731,13 +771,17 @@ do { struct commit *commit = revs->commits->item; - if (commit->object.flags & (UNINTERESTING|SHOWN)) + if (commit->object.flags & SHOWN) + goto next; + if (!(commit->object.flags & BOUNDARY) && + (commit->object.flags & UNINTERESTING)) goto next; if (revs->min_age != -1 && (commit->date > revs->min_age)) goto next; if (revs->max_age != -1 && (commit->date < revs->max_age)) return NULL; - if (revs->no_merges && commit->parents && commit->parents->next) + if (revs->no_merges && + commit->parents && commit->parents->next) goto next; if (revs->prune_fn && revs->dense) { if (!(commit->object.flags & TREECHANGE))
@@ -745,8 +789,12 @@ rewrite_parents(commit); } /* More to go? */ - if (revs->max_count) - pop_most_recent_commit(&revs->commits, SEEN); + if (revs->max_count) { + unsigned flag = SEEN; + if (commit->object.flags & BOUNDARY) + flag |= UNINTERESTING; + pop_most_recent_commit(&revs->commits, flag); + } commit->object.flags |= SHOWN; return commit; next:
diff --git a/revision.h b/revision.h
index 6c2beca..61e6bc9 100644
--- a/revision.h
+++ b/revision.h@@ -6,6 +6,7 @@ #define TREECHANGE (1u<<2) #define SHOWN (1u<<3) #define TMP_MARK (1u<<4) /* for isolated cases; clean after use */ +#define BOUNDARY (1u<<5) struct rev_info;
@@ -32,7 +33,8 @@ blob_objects:1, edge_hint:1, limited:1, - unpacked:1; + unpacked:1, + boundary:1; /* special limits */ int max_count;
From: Paul Mackerras <hidden>
Date: 2016-06-15 22:42:22
Junio C Hamano writes:
How about this alternative patch, then? It turned out to be quite convoluted as I feared.
That's brilliant. Thank you! With the patch to gitk below, the graph display on Linus' example looks much saner. Could you check in your patch to the git.git repository, please? Thanks, Paul.
diff --git a/gitk b/gitk
index 03cd475..1989aa5 100755
--- a/gitk
+++ b/gitk@@ -46,7 +46,7 @@ proc start_rev_list {rlargs} { } if {[catch { set commfd [open [concat | git-rev-list --header $order \ - --parents $rlargs] r] + --parents --boundary $rlargs] r] } err]} { puts stderr "Error executing git-rev-list: $err" exit 1
@@ -114,8 +114,13 @@ proc getcommitlines {commfd} { set start [expr {$i + 1}] set j [string first "\n" $cmit] set ok 0 + set listed 1 if {$j >= 0} { set ids [string range $cmit 0 [expr {$j - 1}]] + if {[string range $ids 0 0] == "-"} { + set listed 0 + set ids [string range $ids 1 end] + } set ok 1 foreach id $ids { if {[string length $id] != 40} {
@@ -133,8 +138,12 @@ proc getcommitlines {commfd} { exit 1 } set id [lindex $ids 0] - set olds [lrange $ids 1 end] - set commitlisted($id) 1 + if {$listed} { + set olds [lrange $ids 1 end] + set commitlisted($id) 1 + } else { + set olds {} + } updatechildren $id $olds set commitdata($id) [string range $cmit [expr {$j + 1}] end] set commitrow($id) $commitidx
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:22
Paul Mackerras [off-list ref] writes:
Junio C Hamano writes:quoted
How about this alternative patch, then? It turned out to be quite convoluted as I feared.That's brilliant. Thank you! With the patch to gitk below, the graph display on Linus' example looks much saner. Could you check in your patch to the git.git repository, please?
The patch I sent was a total mess, and the one in "pu" right now was somewhat cleaned up but was still far suboptimal. **Blush** Most notably, the code from yesterday was re-injecting the parents of the boundary commits into the list marked as UNINTERESTING, which was unnecessary and stupid. This one just pops boundary commits off the list after consuming it. Here is a cleaned-up one for eyeballing. Although I am reasonably sure that this does not affect the way it works when --boundary is not given, I'd pretty much appreciate an independent sanity check on this one. rev-list is so fundamental to git. -- >8 --
diff --git a/rev-list.c b/rev-list.c
index 441c437..f3a989c 100644
--- a/rev-list.c
+++ b/rev-list.c@@ -7,9 +7,9 @@ #include "blob.h" #include "diff.h" #include "revision.h" -/* bits #0-4 in revision.h */ +/* bits #0-5 in revision.h */ -#define COUNTED (1u<<5) +#define COUNTED (1u<<6) static const char rev_list_usage[] = "git-rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
@@ -51,6 +51,8 @@ static void show_commit(struct commit *c printf("%lu ", commit->date); if (commit_prefix[0]) fputs(commit_prefix, stdout); + if (commit->object.flags & BOUNDARY) + putchar('-'); fputs(sha1_to_hex(commit->object.sha1), stdout); if (show_parents) { struct commit_list *parents = commit->parents;
diff --git a/revision.c b/revision.c
index d7678cf..745b0d2 100644
--- a/revision.c
+++ b/revision.c@@ -419,6 +419,27 @@ static void limit_list(struct rev_info * continue; p = &commit_list_insert(commit, p)->next; } + if (revs->boundary) { + list = newlist; + while (list) { + struct commit *commit = list->item; + struct object *obj = &commit->object; + struct commit_list *parent = commit->parents; + if (obj->flags & (UNINTERESTING|BOUNDARY)) { + list = list->next; + continue; + } + while (parent) { + struct commit *pcommit = parent->item; + parent = parent->next; + if (!(pcommit->object.flags & UNINTERESTING)) + continue; + pcommit->object.flags |= BOUNDARY; + p = &commit_list_insert(pcommit, p)->next; + } + list = list->next; + } + } revs->commits = newlist; }
@@ -591,6 +612,10 @@ int setup_revisions(int argc, const char revs->no_merges = 1; continue; } + if (!strcmp(arg, "--boundary")) { + revs->boundary = 1; + continue; + } if (!strcmp(arg, "--objects")) { revs->tag_objects = 1; revs->tree_objects = 1;
@@ -731,13 +756,17 @@ struct commit *get_revision(struct rev_i do { struct commit *commit = revs->commits->item; - if (commit->object.flags & (UNINTERESTING|SHOWN)) + if (commit->object.flags & SHOWN) + goto next; + if (!(commit->object.flags & BOUNDARY) && + (commit->object.flags & UNINTERESTING)) goto next; if (revs->min_age != -1 && (commit->date > revs->min_age)) goto next; if (revs->max_age != -1 && (commit->date < revs->max_age)) return NULL; - if (revs->no_merges && commit->parents && commit->parents->next) + if (revs->no_merges && + commit->parents && commit->parents->next) goto next; if (revs->prune_fn && revs->dense) { if (!(commit->object.flags & TREECHANGE))
@@ -745,8 +774,19 @@ struct commit *get_revision(struct rev_i rewrite_parents(commit); } /* More to go? */ - if (revs->max_count) - pop_most_recent_commit(&revs->commits, SEEN); + if (revs->max_count) { + if (commit->object.flags & BOUNDARY) { + /* this is already uninteresting, + * so there is no point popping its + * parents into the list. + */ + struct commit_list *it = revs->commits; + revs->commits = it->next; + free(it); + } + else + pop_most_recent_commit(&revs->commits, SEEN); + } commit->object.flags |= SHOWN; return commit; next:
diff --git a/revision.h b/revision.h
index 6c2beca..61e6bc9 100644
--- a/revision.h
+++ b/revision.h@@ -6,6 +6,7 @@ #define UNINTERESTING (1u<<1) #define TREECHANGE (1u<<2) #define SHOWN (1u<<3) #define TMP_MARK (1u<<4) /* for isolated cases; clean after use */ +#define BOUNDARY (1u<<5) struct rev_info;
@@ -32,7 +33,8 @@ struct rev_info { blob_objects:1, edge_hint:1, limited:1, - unpacked:1; + unpacked:1, + boundary:1; /* special limits */ int max_count;
From: Alex Riesen <hidden>
Date: 2016-06-15 22:42:22
Paul Mackerras, Wed, Mar 29, 2006 00:51:34 +0200:
Junio C Hamano writes:quoted
How about this alternative patch, then? It turned out to be quite convoluted as I feared.That's brilliant. Thank you! With the patch to gitk below, the graph display on Linus' example looks much saner.
Well... Could you take a look at these screenshots, please? http://home.arcor.de/fork0/bug/gitk1.jpg (this one is BIG, 400k, 2456x949) http://home.arcor.de/fork0/bug/gitk2.jpg http://home.arcor.de/fork0/bug/gitk3.jpg The compressed repository is being uploaded there: http://home.arcor.de/fork0/bug/ggg.tar.bz2 (~6Mb) The old gitk produced a denser graph, which wasn't perfect too, but had a higher count of visible commit titles (and this is two-monitor setup, too).
From: Paul Mackerras <hidden>
Date: 2016-06-15 22:42:22
Alex Riesen writes:
Well... Could you take a look at these screenshots, please? http://home.arcor.de/fork0/bug/gitk1.jpg (this one is BIG, 400k, 2456x949) http://home.arcor.de/fork0/bug/gitk2.jpg http://home.arcor.de/fork0/bug/gitk3.jpg
Yes, I was just looking last night at the part of the git.git graph that you have there in gitk1.jpg. That's an artifact of some changes I made to make sure there was a vertical line segment just before an arrow. The reason for doing that is that Tk 8.4 seems to just punt on drawing an arrow on the end of a diagonal line segment. The old gitk just removed trailing diagonal segments of the line, but I thought I could do better than that. I'll try another approach. Paul.
From: Paul Mackerras <hidden>
Date: 2016-06-15 22:42:22
Alex Riesen writes:
The old gitk produced a denser graph, which wasn't perfect too, but had a higher count of visible commit titles (and this is two-monitor setup, too).
I just pushed a new version which does better on this. Paul.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:22
Paul Mackerras [off-list ref] writes:
Alex Riesen writes:quoted
The old gitk produced a denser graph, which wasn't perfect too, but had a higher count of visible commit titles (and this is two-monitor setup, too).I just pushed a new version which does better on this.
Thanks. Pulled, merged and pushed out..
From: Alex Riesen <hidden>
Date: 2016-06-15 22:42:22
On 3/31/06, Paul Mackerras [off-list ref] wrote:
quoted
The old gitk produced a denser graph, which wasn't perfect too, but had a higher count of visible commit titles (and this is two-monitor setup, too).I just pushed a new version which does better on this.
This one looks reallly much better! Thanks!