[PATCH 4/4] pretty-print: add --pretty=noexpand

It is reasonable for tweak the default output mode for "git log" to
untabify the commit log message, it sometimes may be necessary to
see the output without tab expansion.

Invent a new --pretty option to do this.  Use this to unbreak the
test breakages, where "git shortlog" and output are tested.

Signed-off-by: Junio C Hamano <redacted>
---
 Documentation/pretty-formats.txt | 10 ++++++++++
 Documentation/pretty-options.txt |  2 +-
 commit.h                         |  1 +
 pretty.c                         | 12 +++++++++---
 t/t4201-shortlog.sh              |  6 +++---
 5 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 671cebd..173b932 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt

@@ -39,6 +39,16 @@ This is designed to be as compact as possible.
 
 	      <title line>
 
+	      <full commit message, tab-expanded>
+
+* 'noexpand'
+
+	  commit <sha1>
+	  Author: <author>
+	  Date:   <author date>
+
+	      <title line>
+
 	      <full commit message>
 
 * 'full'

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 4b659ac..7032b1a 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt

@@ -3,7 +3,7 @@
 
 	Pretty-print the contents of the commit logs in a given format,
 	where '<format>' can be one of 'oneline', 'short', 'medium',
-	'full', 'fuller', 'email', 'raw', 'format:<string>'
+	'full', 'fuller', 'email', 'raw', 'noexpand', 'format:<string>'
 	and 'tformat:<string>'.  When '<format>' is none of the above,
 	and has '%placeholder' in it, it acts as if
 	'--pretty=tformat:<format>' were given.

diff --git a/commit.h b/commit.h
index 5d58be0..d511c61 100644
--- a/commit.h
+++ b/commit.h

@@ -126,6 +126,7 @@ enum cmit_fmt {
 	CMIT_FMT_RAW,
 	CMIT_FMT_MEDIUM,
 	CMIT_FMT_DEFAULT = CMIT_FMT_MEDIUM,
+	CMIT_FMT_NOEXPAND,
 	CMIT_FMT_SHORT,
 	CMIT_FMT_FULL,
 	CMIT_FMT_FULLER,

diff --git a/pretty.c b/pretty.c
index 717ceed..8b533dc 100644
--- a/pretty.c
+++ b/pretty.c

@@ -89,6 +89,7 @@ static void setup_commit_formats(void)
 	struct cmt_fmt_map builtin_formats[] = {
 		{ "raw",	CMIT_FMT_RAW,		0 },
 		{ "medium",	CMIT_FMT_MEDIUM,	0 },
+		{ "noexpand",	CMIT_FMT_NOEXPAND,	0 },
 		{ "short",	CMIT_FMT_SHORT,		0 },
 		{ "email",	CMIT_FMT_EMAIL,		0 },
 		{ "fuller",	CMIT_FMT_FULLER,	0 },

@@ -1685,11 +1686,16 @@ static void strbuf_add_tabexpand(struct strbuf *sb,
  * the whole line (without the final newline), after
  * de-tabifying.
  */
-static void pp_handle_indent(struct strbuf *sb, int indent,
+static void pp_handle_indent(struct pretty_print_context *pp,
+			     struct strbuf *sb,
+			     int indent,
 			     const char *line, int linelen)
 {
 	strbuf_addchars(sb, ' ', indent);
-	strbuf_add_tabexpand(sb, line, linelen);
+	if (pp->fmt == CMIT_FMT_MEDIUM)
+		strbuf_add_tabexpand(sb, line, linelen);
+	else
+		strbuf_add(sb, line, linelen);
 }
 
 void pp_remainder(struct pretty_print_context *pp,

@@ -1716,7 +1722,7 @@ void pp_remainder(struct pretty_print_context *pp,
 
 		strbuf_grow(sb, linelen + indent + 20);
 		if (indent)
-			pp_handle_indent(sb, indent, line, linelen);
+			pp_handle_indent(pp, sb, indent, line, linelen);
 		else
 			strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');

diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 987b708..34a9fed 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh

@@ -93,7 +93,7 @@ test_expect_success 'output from user-defined format is re-wrapped' '
 	test_cmp expect log.predictable
 '
 
-test_expect_failure !MINGW 'shortlog wrapping' '
+test_expect_success !MINGW 'shortlog wrapping' '
 	cat >expect <<\EOF &&
 A U Thor (5):
       Test

@@ -114,8 +114,8 @@ EOF
 	test_cmp expect out
 '
 
-test_expect_failure !MINGW 'shortlog from non-git directory' '
-	git log HEAD >log &&
+test_expect_success !MINGW 'shortlog from non-git directory' '
+	git log --pretty=noexpand HEAD >log &&
 	GIT_DIR=non-existing git shortlog -w <log >out &&
 	test_cmp expect out
 '

-- 
2.8.0-rc3-175-g64dcf62

On Thu, Mar 17, 2016 at 4:16 PM, Junio C Hamano [off-list ref] wrote:

It is reasonable for tweak the default output mode for "git log" to
untabify the commit log message, it sometimes may be necessary to
see the output without tab expansion.

Thanks, these all look good to me.

Sorry for not following up, it's just that I'm in the middle of the
kernel merge window and haven't had the time to worry about it.

            Linus

On Thu, Mar 17, 2016 at 04:16:21PM -0700, Junio C Hamano wrote:

It is reasonable for tweak the default output mode for "git log" to
untabify the commit log message, it sometimes may be necessary to
see the output without tab expansion.

Invent a new --pretty option to do this.  Use this to unbreak the
test breakages, where "git shortlog" and output are tested.

Hmm. Isn't "expand tabs" orthogonal to the rest of the pretty format?
That is, couldn't one want "--pretty=fuller, but with tabs expanded"?

I don't personally care much myself, and certainly we don't need to
support "--expand-tabs" for every format until somebody actually wants
them enough to implement it. I just don't want to see us painted into a
corner where we have to support an awkward interface forever (e.g., the
way we had to retrofit the orthogonal "local" concept onto the --date
code).

E.g., start with:

  - only CMIT_FMT_MEDIUM expands tabs (and does so by default)

  - passing --no-expand-tabs suppresses this behavior

  - passing --expand-tabs is an error for now; if people care later,
    they can add support for other formats (naively this is trivial, but
    I suspect there are some corner cases around things like
    --pretty=raw, so unless somebody wants to work on it now, I don't
    think we need to).

-Peff

On Thu, Mar 17, 2016 at 10:08 PM, Jeff King [off-list ref] wrote:

Hmm. Isn't "expand tabs" orthogonal to the rest of the pretty format?
That is, couldn't one want "--pretty=fuller, but with tabs expanded"?

Yeah, you are right, one easily could. And in fact I end up doing
"fuller" myself occasionally, because I check peoples commit
timestamps (some people have a nasty habit of rebasing when they
shouldn't).

So it's not just the medium format that would want detab by default,
it's "full" and "fuller" too (but probably not "raw": that indents the
message too, but the only real reason to use "raw" is for scripting).

So it would probably be better to make it a separate flag, and not tie
it to a particular log format (and just make the log format set the
default).

               Linus

On Thu, Mar 17, 2016 at 10:36:16PM -0700, Linus Torvalds wrote:

On Thu, Mar 17, 2016 at 10:08 PM, Jeff King [off-list ref] wrote:

quoted

Hmm. Isn't "expand tabs" orthogonal to the rest of the pretty format?
That is, couldn't one want "--pretty=fuller, but with tabs expanded"?

Yeah, you are right, one easily could. And in fact I end up doing
"fuller" myself occasionally, because I check peoples commit
timestamps (some people have a nasty habit of rebasing when they
shouldn't).

So it's not just the medium format that would want detab by default,
it's "full" and "fuller" too (but probably not "raw": that indents the
message too, but the only real reason to use "raw" is for scripting).

So it would probably be better to make it a separate flag, and not tie
it to a particular log format (and just make the log format set the
default).

Yeah, I agree with all of that. I didn't want to force anybody to have
to think too hard about corner cases they don't care about (again, as
long as we don't paint ourselves into a corner) but I tend to think that
it makes sense to apply it consistently to all of the stock
human-readable formats (short, medium, full, fuller), but not to "raw"
or "email", and probably not to user-formats.

-Peff