[PATCH 0/4] Flag to add a newline after decorations for --oneline log output

[PATCH 0/4] Flag to add a newline after decorations for --oneline log output

[John Cai via GitGitGadget]

Add a flag --newlineafter to be able to pass in "decorations" to enable a
nicer format for the --oneline output so the commit subjects are aligned
when decorations are printed.

0b96396ef5ff7a3a01e137b3735893c970759dfa (HEAD -> jc/two-line-pretty-decoration, john-cai/jc/two-line-pretty-decoration)
                                         Improve UX for oneline with decorations
e9e5ba39a78c8f5057262d49e261b42a8660d5b9 (origin/master, origin/HEAD, master)
                                         The fifteenth batch
c6fc44e9bf85dc02f6d33b11d9b5d1e10711d125 Merge branch 'ab/test-lib-diff-cleanup'
63ec2297d26155adb0e38745bf2284cd663add8e Merge branch 'ab/fix-make-lint-docs'


original thread in
https://lore.kernel.org/git/CA+55aFwT2HUBzZO8Gpt9tHoJtdRxv9oe3TDoSH5jcEOixRNBXg@mail.gmail.com/T/#t

John Cai (4):
  oneline: parse --newlineafter flag
  oneline: print newline after decorations if flag provided
  oneline: test for --newlineafter feature
  doc: add docs for newlineafter flag

 Documentation/pretty-options.txt |  4 ++++
 builtin/log.c                    | 25 +++++++++++++++++++++++++
 log-tree.c                       | 20 ++++++++++++++++----
 log-tree.h                       |  7 +++++--
 pretty.c                         |  4 ++--
 revision.h                       |  4 ++++
 t/t4205-log-pretty-formats.sh    | 10 ++++++++++
 7 files changed, 66 insertions(+), 8 deletions(-)


base-commit: e9e5ba39a78c8f5057262d49e261b42a8660d5b9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1065%2Fjohn-cai%2Fjc%2Ftwo-line-pretty-decoration-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1065/john-cai/jc/two-line-pretty-decoration-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1065
-- 
gitgitgadget

[PATCH 1/4] oneline: parse --newlineafter flag

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

* revision.h: add newlineafter flag type
* builtin/log.c: parse newlineafter flag with callback

Signed-off-by: John Cai <johncai86@gmail.com>
---
 builtin/log.c | 25 +++++++++++++++++++++++++
 revision.h    |  4 ++++
 2 files changed, 29 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index f75d87e8d7f..3758c73760c 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -53,6 +53,7 @@ static int decoration_given;
 static int use_mailmap_config = 1;
 static const char *fmt_patch_subject_prefix = "PATCH";
 static int fmt_patch_name_max = FORMAT_PATCH_NAME_MAX_DEFAULT;
+static int newlineafter = NEWLINEAFTER_NONE;
 static const char *fmt_pretty;
 
 static const char * const builtin_log_usage[] = {
@@ -132,6 +133,27 @@ static int log_line_range_callback(const struct option *option, const char *arg,
 	return 0;
 }
 
+static int parse_newlineafter(const char *value)
+{
+	if (!strcmp(value, "decorations"))
+		return NEWLINEAFTER_DECORATIONS;
+
+	return NEWLINEAFTER_NONE;
+}
+
+static int option_parse_newlineafter(const struct option *opt,
+				   const char *arg, int unset)
+{
+	int *newlineafter = opt->value;
+
+	if (unset)
+		*newlineafter = NEWLINEAFTER_NONE;
+	else
+		*newlineafter = parse_newlineafter(arg);
+
+	return 0;
+}
+
 static void init_log_defaults(void)
 {
 	init_diff_ui_defaults();
@@ -156,6 +178,7 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 	rev->show_signature = default_show_signature;
 	rev->encode_email_headers = default_encode_email_headers;
 	rev->diffopt.flags.allow_textconv = 1;
+	rev->newlineafter = NEWLINEAFTER_NONE;
 
 	if (default_date_mode)
 		parse_date_format(default_date_mode, &rev->date_mode);
@@ -189,6 +212,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		OPT_CALLBACK('L', NULL, &line_cb, "range:file",
 			     N_("trace the evolution of line range <start>,<end> or function :<funcname> in <file>"),
 			     log_line_range_callback),
+		OPT_CALLBACK(0, "newlineafter", &newlineafter, N_("field"), N_("new line after <field>"), option_parse_newlineafter),
 		OPT_END()
 	};
 
@@ -265,6 +289,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		load_ref_decorations(&decoration_filter, decoration_style);
 	}
 
+	rev->newlineafter = newlineafter;
 	if (rev->line_level_traverse)
 		line_log_init(rev, line_cb.prefix, &line_cb.args);
 
diff --git a/revision.h b/revision.h
index 5578bb4720a..ff0b89c1f40 100644
--- a/revision.h
+++ b/revision.h
@@ -243,6 +243,10 @@ struct rev_info {
 	int		no_inline;
 	int		show_log_size;
 	struct string_list *mailmap;
+	enum {
+			NEWLINEAFTER_NONE,
+			NEWLINEAFTER_DECORATIONS
+		} newlineafter;
 
 	/* Filter by commit log message */
 	struct grep_opt	grep_filter;
-- 
gitgitgadget

[PATCH 2/4] oneline: print newline after decorations if flag provided

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

* log-tree.c: pass in format, oneline, abbrev so format_decorations can
  print a newline with an indentation.
* log-tree.h: adding parameters in header file
* pretty.c: pass in arguments format, oneline to format_decorations call
  but these don't get used in this context

Signed-off-by: John Cai <johncai86@gmail.com>
---
 log-tree.c | 20 ++++++++++++++++----
 log-tree.h |  7 +++++--
 pretty.c   |  4 ++--
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 644893fd8cf..d2f1eeeebf5 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -285,6 +285,9 @@ static void show_name(struct strbuf *sb, const struct name_decoration *decoratio
 void format_decorations_extended(struct strbuf *sb,
 			const struct commit *commit,
 			int use_color,
+			int format,
+			int abbrev_len,
+			int newline,
 			const char *prefix,
 			const char *separator,
 			const char *suffix)
@@ -333,11 +336,16 @@ void format_decorations_extended(struct strbuf *sb,
 	strbuf_addstr(sb, color_commit);
 	strbuf_addstr(sb, suffix);
 	strbuf_addstr(sb, color_reset);
+	if ((format == CMIT_FMT_ONELINE) && newline == 1) {
+		strbuf_addstr(sb, "\n");
+		strbuf_addchars(sb, ' ', abbrev_len);
+	}
 }
 
 void show_decorations(struct rev_info *opt, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
+	int newline = 0;
 
 	if (opt->sources) {
 		char **slot = revision_sources_peek(opt->sources, commit);
@@ -347,7 +355,11 @@ void show_decorations(struct rev_info *opt, struct commit *commit)
 	}
 	if (!opt->show_decorations)
 		return;
-	format_decorations(&sb, commit, opt->diffopt.use_color);
+
+	if (opt->newlineafter == NEWLINEAFTER_DECORATIONS)
+		newline = 1;
+
+	format_decorations(&sb, commit, opt->diffopt.use_color, opt->commit_format, opt->abbrev, newline);
 	fputs(sb.buf, opt->diffopt.file);
 	strbuf_release(&sb);
 }
@@ -623,6 +635,7 @@ void show_log(struct rev_info *opt)
 	int abbrev_commit = opt->abbrev_commit ? opt->abbrev : the_hash_algo->hexsz;
 	const char *extra_headers = opt->extra_headers;
 	struct pretty_print_context ctx = {0};
+	char hex[GIT_MAX_HEXSZ + 1];
 
 	opt->loginfo = NULL;
 	if (!opt->verbose_header) {
@@ -692,9 +705,8 @@ void show_log(struct rev_info *opt)
 
 		if (!opt->graph)
 			put_revision_mark(opt, commit);
-		fputs(find_unique_abbrev(&commit->object.oid,
-					 abbrev_commit),
-		      opt->diffopt.file);
+		opt->abbrev = find_unique_abbrev_r(hex, &commit->object.oid, abbrev_commit);
+		fputs(hex, opt->diffopt.file);
 		if (opt->print_parents)
 			show_parents(commit, abbrev_commit, opt->diffopt.file);
 		if (opt->children.name)
diff --git a/log-tree.h b/log-tree.h
index e7e4641cf83..a401e659d4c 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -19,11 +19,14 @@ int log_tree_commit(struct rev_info *, struct commit *);
 void show_log(struct rev_info *opt);
 void format_decorations_extended(struct strbuf *sb, const struct commit *commit,
 			     int use_color,
+			     int format,
+			     int abbrev_len,
+			     int newline,
 			     const char *prefix,
 			     const char *separator,
 			     const char *suffix);
-#define format_decorations(strbuf, commit, color) \
-			     format_decorations_extended((strbuf), (commit), (color), " (", ", ", ")")
+#define format_decorations(strbuf, commit, color, format, abbrev_len, newline) \
+			     format_decorations_extended((strbuf), (commit), (color), (format), (abbrev_len), (newline), " (", ", ", ")")
 void show_decorations(struct rev_info *opt, struct commit *commit);
 void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			     const char **extra_headers_p,
diff --git a/pretty.c b/pretty.c
index fe95107ae5a..98144deac5c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1385,10 +1385,10 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 		strbuf_addstr(sb, get_revision_mark(NULL, commit));
 		return 1;
 	case 'd':
-		format_decorations(sb, commit, c->auto_color);
+		format_decorations(sb, commit, c->auto_color, c->pretty_ctx->fmt, 0, 0);
 		return 1;
 	case 'D':
-		format_decorations_extended(sb, commit, c->auto_color, "", ", ", "");
+		format_decorations_extended(sb, commit, c->auto_color, c->pretty_ctx->fmt, 0, 0, "", ", ", "");
 		return 1;
 	case 'S':		/* tag/branch like --source */
 		if (!(c->pretty_ctx->rev && c->pretty_ctx->rev->sources))
-- 
gitgitgadget

[PATCH 3/4] oneline: test for --newlineafter feature

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t4205-log-pretty-formats.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 5865daa8f8d..847426f8af0 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -576,6 +576,16 @@ test_expect_success 'clean log decoration' '
 	test_cmp expected actual1
 '
 
+test_expect_success 'oneline with --newlineafter=decorations' '
+	git checkout -b newlineafter &&
+	>baz &&
+	git add baz &&
+	git commit -m "decorations" &&
+	git log --pretty=format:"%h%d%n%>(18) %s%n" --max-count=1 >expect &&
+	git log --oneline --decorate --max-count=1 --newlineafter=decorations >actual &&
+	test_cmp expect actual
+'
+
 cat >trailers <<EOF
 Signed-off-by: A U Thor <author@example.com>
 Acked-by: A U Thor <author@example.com>
-- 
gitgitgadget

[PATCH 4/4] doc: add docs for newlineafter flag

[John Cai via GitGitGadget]

From: John Cai <johncai86@gmail.com>

Provide documentation for --newlineafter flag

Signed-off-by: John Cai <johncai86@gmail.com>
---
 Documentation/pretty-options.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index b3af8506086..7de51c660fe 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -32,6 +32,10 @@ people using 80-column terminals.
 	This is a shorthand for "--pretty=oneline --abbrev-commit"
 	used together.
 
+--newlineafter[=(decorations)]
+	Used in conjunction with --pretty=oneline to add a line break after
+	decorations
+
 --encoding=<encoding>::
 	Commit objects record the character encoding used for the log message
 	in their encoding header; this option can be used to tell the
-- 
gitgitgadget

Re: [PATCH 0/4] Flag to add a newline after decorations for --oneline log output

[Christian Couder]

On Fri, Oct 29, 2021 at 11:17 PM John Cai via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> Add a flag --newlineafter to be able to pass in "decorations" to enable a
> nicer format for the --oneline output so the commit subjects are aligned
> when decorations are printed.

I wonder if --newlinebefore or --newlinefor rather than --newlineafter
would be better. It seems to me that it would be easier for users to
guess what the result will look like with --newlinefor.

Another possibly more generic solution would be something like
--format-field=<field>:<format> where, in the output, the field
<field> (which corresponds to the "%(<field>)" format) would be
replaced by <format> which should contain "%(<field>)". For example
`--format-field=decorations:'[[%(decorations)]]\n'` would enclose the
decorations using [[...]] and would add a newline after them.

Also it would be nice if this could be extended to other fields and to
formats other than "oneline". You might want to discuss a bit about
how it could be done with generic code.

> 0b96396ef5ff7a3a01e137b3735893c970759dfa (HEAD -> jc/two-line-pretty-decoration, john-cai/jc/two-line-pretty-decoration)
>                                          Improve UX for oneline with decorations
> e9e5ba39a78c8f5057262d49e261b42a8660d5b9 (origin/master, origin/HEAD, master)
>                                          The fifteenth batch
> c6fc44e9bf85dc02f6d33b11d9b5d1e10711d125 Merge branch 'ab/test-lib-diff-cleanup'
> 63ec2297d26155adb0e38745bf2284cd663add8e Merge branch 'ab/fix-make-lint-docs'

Not sure why the above lines are included in your cover letter.

> original thread in
> https://lore.kernel.org/git/CA+55aFwT2HUBzZO8Gpt9tHoJtdRxv9oe3TDoSH5jcEOixRNBXg@mail.gmail.com/T/#t

It would be nice if you could summarize the threads a bit in this
cover letter, and tell a bit about how your design evolved from what
was discussed.

> John Cai (4):
>   oneline: parse --newlineafter flag
>   oneline: print newline after decorations if flag provided
>   oneline: test for --newlineafter feature
>   doc: add docs for newlineafter flag

The 2 last patches look very small and might want to be squashed into
the patch that introduces the --newlineafter flag.

Thanks!

Re: [PATCH 0/4] Flag to add a newline after decorations for --oneline log output

[John Cai]

On Fri, Nov 12, 2021 at 05:27:45PM +0100, Christian Couder wrote:
> On Fri, Oct 29, 2021 at 11:17 PM John Cai via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > Add a flag --newlineafter to be able to pass in "decorations" to enable a
> > nicer format for the --oneline output so the commit subjects are aligned
> > when decorations are printed.
> 
> I wonder if --newlinebefore or --newlinefor rather than --newlineafter
> would be better. It seems to me that it would be easier for users to
> guess what the result will look like with --newlinefor.
> 
> Another possibly more generic solution would be something like
> --format-field=<field>:<format> where, in the output, the field
> <field> (which corresponds to the "%(<field>)" format) would be
> replaced by <format> which should contain "%(<field>)". For example
> `--format-field=decorations:'[[%(decorations)]]\n'` would enclose the
> decorations using [[...]] and would add a newline after them.
> 
> Also it would be nice if this could be extended to other fields and to
> formats other than "oneline". You might want to discuss a bit about
> how it could be done with generic code.
> 
> > 0b96396ef5ff7a3a01e137b3735893c970759dfa (HEAD -> jc/two-line-pretty-decoration, john-cai/jc/two-line-pretty-decoration)
> >                                          Improve UX for oneline with decorations
> > e9e5ba39a78c8f5057262d49e261b42a8660d5b9 (origin/master, origin/HEAD, master)
> >                                          The fifteenth batch
> > c6fc44e9bf85dc02f6d33b11d9b5d1e10711d125 Merge branch 'ab/test-lib-diff-cleanup'
> > 63ec2297d26155adb0e38745bf2284cd663add8e Merge branch 'ab/fix-make-lint-docs'
> 
> Not sure why the above lines are included in your cover letter.
> 
> > original thread in
> > https://lore.kernel.org/git/CA+55aFwT2HUBzZO8Gpt9tHoJtdRxv9oe3TDoSH5jcEOixRNBXg@mail.gmail.com/T/#t
> 
> It would be nice if you could summarize the threads a bit in this
> cover letter, and tell a bit about how your design evolved from what
> was discussed.
> 
> > John Cai (4):
> >   oneline: parse --newlineafter flag
> >   oneline: print newline after decorations if flag provided
> >   oneline: test for --newlineafter feature
> >   doc: add docs for newlineafter flag
> 
> The 2 last patches look very small and might want to be squashed into
> the patch that introduces the --newlineafter flag.
> 
> Thanks!

There was a discussion in https://lore.kernel.org/git/CA+55aFwT2HUBzZO8Gpt9tHoJtdRxv9oe3TDoSH5jcEOixRNBXg@mail.gmail.com/T/#t

about improving the --oneline output with decorations. To summarize, currently, when
decorations are printed with --oneline the subject message ends up misaligned like so:

```
7140c4988f t/lib-git.sh: fix ACL-related permissions failure
88d915a634 (jc-test-peff-revlist-patch) A few fixes before -rc2
9cc14a5b5d Sync with maint
5fbd2fc599 (origin/maint) Merge branch 'vd/pthread-setspecific-g11-fix' into maint
494cb27e57 Merge branch 'ma/doc-git-version' into maint
```

It would improve the user experience of this format if we could allow a newline after
the decorations so that the logs are formatted in the following manner:

```
7140c4988f t/lib-git.sh: fix ACL-related permissions failure
88d915a634 (jc-test-peff-revlist-patch)
           A few fixes before -rc2
9cc14a5b5d Sync with maint
5fbd2fc599 (origin/maint)
           Merge branch 'vd/pthread-setspecific-g11-fix' into maint
494cb27e57 Merge branch 'ma/doc-git-version' into maint
```

In order to accomplish this, we can add a new flag called --format-field=<format>.
<format> is similar to what is passed into --format.

For example, if --format-field=[[%d]]%n is used, the --oneline format will print
decorations enclosed with [[ ]] and a newline after it.

This would be convenient when one wants to modify just one field of the
log output.


cc: Christian Couder <christian.couder@gmail.com>