Re: [PATCH] log: allow --graph and --show-linear-break used together

[Duy Nguyen <pclouds@gmail.com>]

On Fri, Apr 08, 2016 at 01:21:07PM +0200, Jan Kundrát wrote:
> From 1ac6bb7c31652835d3d046c82e423f0cea6e0904 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Jan=20Kundr=C3=A1t?= <jkt@kde.org>
> Date: Fri, 8 Apr 2016 13:06:31 +0200
> Subject: [PATCH] log: allow --graph and --show-linear-break used together
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> There was no visual break when --graph --oneline was used with several
> history trees with disjoint parents.
> 
> The original reasoning given in commit 1b32decef was probably that a
> graph already conveys enough information to indicate that there's no
> parent-child relation between these two commits. However, this is not
> the case when a single-line prettification of a log is used, in which
> case the commits are displayed on top of each other with no space for
> that graph line to show the separation.
> 
> It might be interesting to change the actual printing of the break_bar
> to print fewer line breaks, but this simple approach works and is IMHO
> not that bad visually, anyway.

How about something like this? It does not break the graph, which is
probably better visually, although some may say breaking the graph
_is_ better in this particular case.

I also try to fix the extra blank lines in --oneline mode (with or
without --graph). That's definitely an improvement.

-- 8< --
diff --git a/graph.c b/graph.c
index 1350bdd..deafdd4 100644
--- a/graph.c
+++ b/graph.c
@@ -1226,6 +1226,14 @@ void graph_show_padding(struct git_graph *graph)
 	strbuf_release(&msgbuf);
 }
 
+void graph_show_vertical_bars(struct git_graph *graph)
+{
+	int saved_width = graph->width;
+	graph->width = 0;	/* no extra padding */
+	graph_show_padding(graph);
+	graph->width = saved_width;
+}
+
 int graph_show_remainder(struct git_graph *graph)
 {
 	struct strbuf msgbuf = STRBUF_INIT;
diff --git a/graph.h b/graph.h
index 0be62bd..5e55425 100644
--- a/graph.h
+++ b/graph.h
@@ -91,6 +91,8 @@ void graph_show_oneline(struct git_graph *graph);
  */
 void graph_show_padding(struct git_graph *graph);
 
+void graph_show_vertical_bars(struct git_graph *graph);
+
 /*
  * If the graph is non-NULL, print the rest of the history graph for this
  * commit to stdout.  Does not print a terminating newline on the last line.
diff --git a/log-tree.c b/log-tree.c
index 60f9839..61d7cbd 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -858,6 +858,22 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 	return showed_log;
 }
 
+static void show_break_bar(struct rev_info *opt)
+{
+	if (opt->graph) {
+		if (opt->commit_format != CMIT_FMT_ONELINE) {
+			graph_show_vertical_bars(opt->graph);
+			putchar('\n');
+		}
+		graph_show_vertical_bars(opt->graph);
+		printf("%s\n", opt->break_bar);
+	} else {
+		if (opt->commit_format != CMIT_FMT_ONELINE)
+			putchar('\n');
+		printf("%s\n", opt->break_bar);
+	}
+}
+
 int log_tree_commit(struct rev_info *opt, struct commit *commit)
 {
 	struct log_info log;
@@ -871,7 +887,7 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 		return line_log_print(opt, commit);
 
 	if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)
-		printf("\n%s\n", opt->break_bar);
+		show_break_bar(opt);
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
 		log.parent = NULL;
@@ -879,7 +895,7 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 		shown = 1;
 	}
 	if (opt->track_linear && !opt->linear && opt->reverse_output_stage)
-		printf("\n%s\n", opt->break_bar);
+		show_break_bar(opt);
 	opt->loginfo = NULL;
 	maybe_flush_or_die(stdout, "stdout");
 	return shown;
-- 8< --


> 
> Signed-off-by: Jan Kundr??t <jkt@kde.org>
> ---
>  Documentation/rev-list-options.txt | 5 +++--
>  revision.c                         | 4 ++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 4f009d4..c5e487c 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -807,8 +807,9 @@ This implies the `--topo-order` option by default, but the
>  `--date-order` option may also be specified.
>  
>  --show-linear-break[=<barrier>]::
> -	When --graph is not used, all history branches are flattened
> -	which can make it hard to see that the two consecutive commits
> +	If the history is flattened, such as when --graph is not used
> +	or if --graph is combined with --oneline to produce a compact
> +	view, it can be hard to see that the two consecutive commits
>  	do not belong to a linear branch. This option puts a barrier
>  	in between them in that case. If `<barrier>` is specified, it
>  	is the string that will be shown instead of the default one.
> diff --git a/revision.c b/revision.c
> index 8b2dfe3..809c43e 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1864,6 +1864,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  		   starts_with(arg, "--show-linear-break=")) {
>  		if (starts_with(arg, "--show-linear-break="))
>  			revs->break_bar = xstrdup(arg + 20);
> +		else if (revs->graph)
> +			revs->break_bar = "  ..........\n";
>  		else
>  			revs->break_bar = "                    ..........";
>  		revs->track_linear = 1;
> @@ -1993,8 +1995,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  			unkv[(*unkc)++] = arg;
>  		return opts;
>  	}
> -	if (revs->graph && revs->track_linear)
> -		die("--show-linear-break and --graph are incompatible");
>  
>  	return 1;
>  }
> -- 
> 2.7.3
>