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

[Junio C Hamano <gitster@pobox.com>]

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 <gitster@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 &&
+		    graph->prev_commit_index == graph->commit_index)
+			graph->need_post_root_gap = 1;
+	}
 }
 
 static int graph_is_mapping_correct(struct git_graph *graph)
@@ -814,6 +825,11 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)
 	int seen_this = 0;
 	int i, chars_written;
 
+	if (graph->need_post_root_gap) {
+		graph->need_post_root_gap = 0;
+		strbuf_addch(sb, '\n');
+	}
+
 	/*
 	 * Output the row containing this commit
 	 * Iterate up to and including graph->num_columns,
diff --git a/t/t6016-rev-list-graph-simplify-history.sh b/t/t6016-rev-list-graph-simplify-history.sh
index f7181d1..ca53a80 100755
--- a/t/t6016-rev-list-graph-simplify-history.sh
+++ b/t/t6016-rev-list-graph-simplify-history.sh
@@ -264,4 +264,38 @@ test_expect_success '--graph --boundary ^C3' '
 	test_cmp expected actual
 	'
 
+one_independent_branch () {
+	git checkout --orphan root$1 A1 &&
+	test_commit root_$1 &&
+	test_commit then_$1 &&
+	test_commit further_$1
+}
+
+test_expect_success 'multi-root setup' '
+	one_independent_branch 0 &&
+	one_independent_branch 1 &&
+	one_independent_branch 2 &&
+
+	git checkout -b merge210 root2 &&
+	test_tick &&
+	git merge -s ours root1 &&
+	test_tick &&
+	git merge -s ours root0
+'
+
+test_expect_success 'multi-root does not emit unnecessary post-root gap' '
+	git log --oneline --graph >actual &&
+	! grep "^$" actual
+'
+
+test_expect_success 'multi-root does show necessary post-root gap' '
+	git log --oneline --graph root0 root1 root2 >actual &&
+	test $(grep -c "^$" actual) = 2
+'
+
+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
+'
+
 test_done
-- 
1.8.5.2-297-g3e57c29