Re: [RFH/PATCH] graph: give an extra gap after showing root commit

2014-01-03 Thread Thomas Rast
Hi Junio,

I briefly looked at d84a3da (jc/graph-post-root-gap) in pu, and have
this nit:

 diff --git a/t/t6016-rev-list-graph-simplify-history.sh 
 b/t/t6016-rev-list-graph-simplify-history.sh
 [...]
 +one_independent_branch () {
 + git checkout --orphan root$1 A1 
 + test_commit root_$1 

The naming of root0 etc. makes the test below rather confusing to read,
because test_commit root_0 also creates a tag called root_0.  So you set
up history that has a tag root_0 that points *only* at the root, and a
branch root0 that includes two more commits.

 +test_expect_failure 'multi-root does show necessary post-root gap' '
 + sed -e s/ #$/ / expect -\EOF 
 + * further_2
 + * then_2
 + * root_2
 +   * further_1
 +   * then_1
 +   * root_1
 + * further_0
 + * then_0
 + * root_0
 + EOF
 + git log --graph --format=%s root0 root1 root2 actual 
 + test_cmp expect actual
 +'

-- 
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFH/PATCH] graph: give an extra gap after showing root commit

2013-12-20 Thread Junio C Hamano
With a history with more than one root commit, if a root commit
falls on one display column and another commit that is unrelated to
that root's history is shown on the next line on the same column,
the resulting graph would appear as if the latter is a parent of the
former, like this (there are two histories a1-a2  b1-b2):

--
$ git log --graph --oneline a2 b2
* a2
* a1
* b2
* b1
--

which is misleading.  b2 is a tip of a history unrelated to the
history that has a1 as a root.

Force a gap line immediately before showing next unrelated commit if
we showed a root from one history, to make the above display look
like this instead:

--
$ git log --graph --oneline a2 b2
* a2
* a1

* b2
* b1
--

but do not waste line when we do not have to.  E.g. with a history
that merges a2 and b2,

--
$ git log --graph --oneline m
* Merge a2 and b2
|\
| * a2
| * a1
* b2
* b1
--

there is no need to show an extra blank line after showing a1, as
it is clear that it has no parents.

This takes inspiration from Milton Soares Filho's graph.c: mark
root commit differently from a few months ago ($gmane/236708),
which tried to show a root commit as 'x', but only if it would have
been shown as '*' in the original, but uses a different approach so
that a root that may have been painted differently from '*'
(e.g. '' for left root) can also be made distinguishable.

Signed-off-by: Junio C Hamano gits...@pobox.com

---

 Note that this still does not work very well for --boundary case
 (see the last test added to t6016).

 It may actually make sense to force the next commit after showing
 a root always occupy a different column, instead of wasting a blank
 line.  If we did so, the output from the first example may look like
 this:

--
$ git log --graph --oneline a2 b2
* a2
* a1
  * b2
  * b1
--

 or it may even look like this:

--
$ git log --graph --oneline a2 b2
* a2
* a1
  * b2
 /
* b1
--

 I tried to follow graph_update_columns() logic but gave up for now;
 avoiding to place a commit whose descendant has already been seen
 to the same column as the root commit we are processing there is
 easy, but the current commit may not yet be in columns[], and
 graph_output_commit_line() needs to show the current commit beyond
 the end of the columns[] list in such a case, so teaching
 graph_update_columns() to tweak placement of the next line is not
 sufficient.

 graph.c| 20 --
 t/t6016-rev-list-graph-simplify-history.sh | 34 ++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/graph.c b/graph.c
index b24d04c..2c3f141 100644
--- a/graph.c
+++ b/graph.c
@@ -187,6 +187,10 @@ struct git_graph {
 * stored as an index into the array column_colors.
 */
unsigned short default_column_color;
+   /* The last one was a root and we haven't emitted an extra blank */
+   unsigned need_post_root_gap : 1;
+   /* Are we looking at the root? */
+   unsigned is_root : 1;
 };
 
 static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, 
void *data)
@@ -205,7 +209,7 @@ static struct strbuf *diff_output_prefix_callback(struct 
diff_options *opt, void
 
 struct git_graph *graph_init(struct rev_info *opt)
 {
-   struct git_graph *graph = xmalloc(sizeof(struct git_graph));
+   struct git_graph *graph = xcalloc(1, sizeof(struct git_graph));
 
if (!column_colors)
graph_set_column_colors(column_colors_ansi,
@@ -552,11 +556,14 @@ static void graph_update_columns(struct git_graph *graph)
 void graph_update(struct git_graph *graph, struct commit *commit)
 {
struct commit_list *parent;
+   int was_root = graph-is_root;
 
/*
 * Set the new commit
 */
graph-commit = commit;
+   graph-is_root = !commit-parents;
+   graph-need_post_root_gap = 0;
 
/*
 * Count how many interesting parents this commit has
@@ -607,8 +614,12 @@ void graph_update(struct git_graph *graph, struct commit 
*commit)
else if (graph-num_parents = 3 
 graph-commit_index  (graph-num_columns - 1))
graph-state = GRAPH_PRE_COMMIT;
-   else
+   else {
graph-state = GRAPH_COMMIT;
+   if (was_root 
+  

Re: [RFH/PATCH] graph: give an extra gap after showing root commit

2013-12-20 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

  Note that this still does not work very well for --boundary case
  (see the last test added to t6016).
 ...
 +test_expect_failure 'multi-root does not emit unnecessary post-root gap' '
 + git log --oneline --graph merge210~1...merge210~1^2~2 actual 
 + ! grep ^$ actual
 +'

Obviously, this needs to be

git log --oneline --graph --boundary merge210~1...merge210~1^2~2 actual 

for it to fail.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html