Re: [RFC PATCH] show decorations at the end of the line

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Feb 20, 2017, 01:48 UTC
Message-ID: <CA+55aFwdUxCvmi28T3yn1K4rqn2bZmJBdTRr7tSbMa-g5izHbw@mail.gmail.com>
In-Reply-To: <20170220004648.c2zz6bm2hylvep6x@sigill.intra.peff.net>

On Sun, Feb 19, 2017 at 4:46 PM, Jeff King <peff@peff.net> wrote:

Show 5 quoted lines

>
> I think there are two potential patches:
>
>   1. Add a custom-format placeholder for the --source value.
>      This is an obvious improvement that doesn't hurt anyone.

Right.

>   2. Switch --decorate to the end by default, but _not_ --source.

.. and in fact the whole "--source" printing should not even have been mixed up with the decorations.

So (2) is actually easy to fix: just don't mix "show_source()" with "show_decorations()", because they are totally different things to begin with.

That source showing should never have been in "show_decorations()" in the first place. It just happened to be a convenient place for it.

So this attached patch is just my original patch updated to split up "show_source()" from "show_decorations()", and show it where it used to be.

Maybe this works for Junio's alias?

              Linus

From 97abab56fed451476f6ec676346b1e66001ef864 Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sat, 11 Feb 2017 10:03:33 -0800
Subject: [PATCH] show decorations at the end of the line

So I use "--show-decorations" all the time because I find it very useful to see where the origin branch is, where tags are etc. In fact, my global git config file has

    [log]
        decorate = auto

in it, so that I don't have to type it out all the time when I just do my usual 'git log". It's lovely.

However, it does make one particular case uglier: with commit decorations, the "oneline" commit format ends up being not very pretty:

    [torvalds@i7 git]$ git log --oneline -10
    3f07dac29 (HEAD -> master) pathspec: don't error out on  all-exclusionary pathspec patterns
    ca4a562f2 pathspec magic: add '^' as alias for '!'
    02555c1b2 ls-remote: add "--diff" option to show only refs that differ
    6e3a7b339 (tag: v2.12.0-rc0, origin/master, origin/HEAD) Git 2.12-rc0
    fafca0f72 Merge branch 'cw/log-updates-for-all-refs-really'
    74dee5cfa Merge branch 'pl/complete-diff-submodule-diff'
    36acf4123 Merge branch 'rs/object-id'
    ecc486b1f Merge branch 'js/re-running-failed-tests'
    4ba6bb2d1 Merge branch 'sb/submodule-update-initial-runs-custom-script'
    5348021c6 Merge branch 'sb/submodule-recursive-absorb'

and note how the decoration comes right after the shortened commit hash, breaking up the alignment of the messages.

The above doesn't show it with the colorization: I also have

    [color]
        ui=auto

so on my terminal the decoration is also nicely colorized which makes it much more obvious, it's not as obvious in this message.

The oneline message handling is already pretty special, this makes it even more special by putting the decorations at the end of the line:

    3f07dac29 pathspec: don't error out on all-exclusionary pathspec patterns (HEAD -> master)
    ca4a562f2 pathspec magic: add '^' as alias for '!'
    02555c1b2 ls-remote: add "--diff" option to show only refs that differ
    6e3a7b339 Git 2.12-rc0 (tag: v2.12.0-rc0, origin/master, origin/HEAD)
    fafca0f72 Merge branch 'cw/log-updates-for-all-refs-really'
    74dee5cfa Merge branch 'pl/complete-diff-submodule-diff'
    36acf4123 Merge branch 'rs/object-id'
    ecc486b1f Merge branch 'js/re-running-failed-tests'
    4ba6bb2d1 Merge branch 'sb/submodule-update-initial-runs-custom-script'
    5348021c6 Merge branch 'sb/submodule-recursive-absorb'

which looks a lot better (again, this is all particularly noticeable with colorization).

NOTE! There's a very special case for "git log --oneline -g" that shows the reflogs as oneliners, and this does *not* fix that special case. It's a lot more involved and relies on the exact show_reflog_message() implementation, so I left the format for that alone, along with a comment about how it's not at the end of line.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 builtin/rev-list.c |  1 +
 log-tree.c         | 17 ++++++++++++++---
 log-tree.h         |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0aa93d589..8833f029a 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -107,6 +107,7 @@ static void show_commit(struct commit *commit, void *data)
 			children = children->next;
 		}
 	}
+	show_source(revs, commit);
 	show_decorations(revs, commit);
 	if (revs->commit_format == CMIT_FMT_ONELINE)
 		putchar(' ');
diff --git a/log-tree.c b/log-tree.c
index 8c2415747..9ca3e8c1c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -279,12 +279,16 @@ void format_decorations_extended(struct strbuf *sb,
 	strbuf_addstr(sb, color_reset);
 }
 
+void show_source(struct rev_info *opt, struct commit *commit)
+{
+	if (opt->show_source && commit->util)
+		fprintf(opt->diffopt.file, "\t%s", (char *) commit->util);
+}
+
 void show_decorations(struct rev_info *opt, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
 
-	if (opt->show_source && commit->util)
-		fprintf(opt->diffopt.file, "\t%s", (char *) commit->util);
 	if (!opt->show_decorations)
 		return;
 	format_decorations(&sb, commit, opt->diffopt.use_color);
@@ -556,6 +560,7 @@ void show_log(struct rev_info *opt)
 			show_parents(commit, abbrev_commit, opt->diffopt.file);
 		if (opt->children.name)
 			show_children(opt, commit, abbrev_commit);
+		show_source(opt, commit);
 		show_decorations(opt, commit);
 		if (opt->graph && !graph_is_commit_finished(opt->graph)) {
 			putc('\n', opt->diffopt.file);
@@ -622,10 +627,14 @@ void show_log(struct rev_info *opt)
 			       find_unique_abbrev(parent->object.oid.hash,
 						  abbrev_commit));
 		fputs(diff_get_color_opt(&opt->diffopt, DIFF_RESET), opt->diffopt.file);
-		show_decorations(opt, commit);
+		show_source(opt, commit);
 		if (opt->commit_format == CMIT_FMT_ONELINE) {
+			/* Not at end of line, but.. */
+			if (opt->reflog_info)
+				show_decorations(opt, commit);
 			putc(' ', opt->diffopt.file);
 		} else {
+			show_decorations(opt, commit);
 			putc('\n', opt->diffopt.file);
 			graph_show_oneline(opt->graph);
 		}
@@ -716,6 +725,8 @@ void show_log(struct rev_info *opt)
 		opt->missing_newline = 0;
 
 	graph_show_commit_msg(opt->graph, opt->diffopt.file, &msgbuf);
+	if (ctx.fmt == CMIT_FMT_ONELINE)
+		show_decorations(opt, commit);
 	if (opt->use_terminator && !commit_format_is_empty(opt->commit_format)) {
 		if (!opt->missing_newline)
 			graph_show_padding(opt->graph);
diff --git a/log-tree.h b/log-tree.h
index c8116e60c..33d415820 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -20,6 +20,7 @@ void format_decorations_extended(struct strbuf *sb, const struct commit *commit,
 			     const char *suffix);
 #define format_decorations(strbuf, commit, color) \
 			     format_decorations_extended((strbuf), (commit), (color), " (", ", ", ")")
+void show_source(struct rev_info *opt, struct commit *commit);
 void show_decorations(struct rev_info *opt, struct commit *commit);
 void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			     const char **subject_p,

-- 
2.12.0.rc2.4.g97abab56f