* Re: [PATCH] log: allow --graph and --show-linear-break used together
2016-04-08 11:21 [PATCH] log: allow --graph and --show-linear-break used together Jan Kundrát
@ 2016-04-08 12:43 ` Duy Nguyen
2016-04-08 17:11 ` Junio C Hamano
1 sibling, 0 replies; 3+ messages in thread
From: Duy Nguyen @ 2016-04-08 12:43 UTC (permalink / raw)
To: Jan Kundrát; +Cc: git
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
>
^ permalink raw reply related [flat|nested] 3+ messages in thread