[PATCH] pretty-print: de-tabify indented logs to make things line up properly

[PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Linus Torvalds]

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 16 Mar 2016 09:15:53 -0700
Subject: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

This should all line up:

  Column 1	Column 2
  --------	--------
  A		B
  ABCD		EFGH
  SPACES        Instead of Tabs

Even with multi-byte UTF8 characters:

  Column 1	Column 2
  --------	--------
  Ä		B
  åäö		100
  A Møøse	once bit my sister..

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

This seems to work for me, and while there is some cost, it's minimal. 
Doing a "git log > /dev/null" of the current git tree is about 1% slower 
because of the tab-finding. A tree with a lot of tabs in the commit 
messages would be more noticeable, because then you actually end up 
hitting the whole "how wide is this" issue.

(But if the tabs are all at the beginning of a line, you'd still be ok 
and avoid the utf8 width calculations).

Comments?

 pretty.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 2 deletions(-)

diff --git a/pretty.c b/pretty.c
index 92b2870a7eab..0b40457f99f0 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1629,6 +1629,76 @@ void pp_title_line(struct pretty_print_context *pp,
 	strbuf_release(&title);
 }
 
+static int pp_utf8_width(const char *start, const char *end)
+{
+	int width = 0;
+	size_t remain = end - start;
+
+	while (remain) {
+		int n = utf8_width(&start, &remain);
+		if (n < 0 || !start)
+			return -1;
+		width += n;
+	}
+	return width;
+}
+
+/*
+ * pp_handle_indent() prints out the intendation, and
+ * perhaps the whole line (without the final newline)
+ *
+ * Why "perhaps"? If there are tabs in the indented line
+ * it will print it out in order to de-tabify the line.
+ *
+ * But if there are no tabs, we just fall back on the
+ * normal "print the whole line".
+ */
+static int pp_handle_indent(struct strbuf *sb, int indent,
+			     const char *line, int linelen)
+{
+	const char *tab;
+
+	strbuf_addchars(sb, ' ', indent);
+
+	tab = memchr(line, '\t', linelen);
+	if (!tab)
+		return 0;
+
+	do {
+		int width = pp_utf8_width(line, tab);
+
+		/*
+		 * If it wasn't well-formed utf8, or it
+		 * had characters with badly defined
+		 * width (control characters etc), just
+		 * give up on trying to align things.
+		 */
+		if (width < 0)
+			break;
+
+		/* Output the data .. */
+		strbuf_add(sb, line, tab - line);
+
+		/* .. and the de-tabified tab */
+		strbuf_addchars(sb, ' ', 8-(width & 7));
+
+		/* Skip over the printed part .. */
+		linelen -= 1+tab-line;
+		line = tab + 1;
+
+		/* .. and look for the next tab */
+		tab = memchr(line, '\t', linelen);
+	} while (tab);
+
+	/*
+	 * Print out everything after the last tab without
+	 * worrying about width - there's nothing more to
+	 * align.
+	 */
+	strbuf_add(sb, line, linelen);
+	return 1;
+}
+
 void pp_remainder(struct pretty_print_context *pp,
 		  const char **msg_p,
 		  struct strbuf *sb,
@@ -1652,8 +1722,10 @@ void pp_remainder(struct pretty_print_context *pp,
 		first = 0;
 
 		strbuf_grow(sb, linelen + indent + 20);
-		if (indent)
-			strbuf_addchars(sb, ' ', indent);
+		if (indent) {
+			if (pp_handle_indent(sb, indent, line, linelen))
+				linelen = 0;
+		}
 		strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
 	}
-- 
2.8.0.rc2

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Linus Torvalds]

On Wed, Mar 16, 2016 at 9:29 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> This should all line up:
>
>   Column 1      Column 2
>   --------      --------
>   A             B
>   ABCD          EFGH
>   SPACES        Instead of Tabs
>
> Even with multi-byte UTF8 characters:
>
>   Column 1      Column 2
>   --------      --------
>   Ä             B
>   åäö           100
>   A Møøse       once bit my sister..

So with current git, it looks like this for me:

      Column 1  Column 2
      --------  --------
      A         B
      ABCD              EFGH
      SPACES        Instead of Tabs

    Even with multi-byte UTF8 characters:

      Column 1  Column 2
      --------  --------
      Ä         B
      åäö               100
      A Møøse   once bit my sister..

and with that patch it looks much better:

      Column 1      Column 2
      --------      --------
      A             B
      ABCD          EFGH
      SPACES        Instead of Tabs

    Even with multi-byte UTF8 characters:

      Column 1      Column 2
      --------      --------
      Ä             B
      åäö           100
      A Møøse       once bit my sister..

(of course, to see that assumes that you have a fixed-sized font in
your mail viewer, and that the spacing didn't get screwed up by my
cut-and-paste).

             Linus

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Wed, 16 Mar 2016 09:15:53 -0700
> Subject: [PATCH] pretty-print: de-tabify indented logs to make things line up properly
>
> This should all line up:
>
>   Column 1	Column 2
>   --------	--------
>   A		B
>   ABCD		EFGH
>   SPACES        Instead of Tabs
>
> Even with multi-byte UTF8 characters:
>
>   Column 1	Column 2
>   --------	--------
>   Ä		B
>   åäö		100
>   A Møøse	once bit my sister..
>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>
> This seems to work for me, and while there is some cost, it's minimal. 
> Doing a "git log > /dev/null" of the current git tree is about 1% slower 
> because of the tab-finding. A tree with a lot of tabs in the commit 
> messages would be more noticeable, because then you actually end up 
> hitting the whole "how wide is this" issue.
>
> (But if the tabs are all at the beginning of a line, you'd still be ok 
> and avoid the utf8 width calculations).
>
> Comments?

I stared at it for a while, and didn't spot anything wrong with it.

I did wonder about two things, though:

 (1) if turning your "preparation; do { ... } while()" into
     "while () { }" would make the result a bit easier to read;

 (2) if we can somehow eliminate duplication of "tab + 1" (spelled
     differently on the previous line as "1+tab"), the end result
     may get easier to follow.

but both are minor.

>  pretty.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 74 insertions(+), 2 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index 92b2870a7eab..0b40457f99f0 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1629,6 +1629,76 @@ void pp_title_line(struct pretty_print_context *pp,
>  	strbuf_release(&title);
>  }
>  
> +static int pp_utf8_width(const char *start, const char *end)
> +{
> +	int width = 0;
> +	size_t remain = end - start;
> +
> +	while (remain) {
> +		int n = utf8_width(&start, &remain);
> +		if (n < 0 || !start)
> +			return -1;
> +		width += n;
> +	}
> +	return width;
> +}
> +
> +/*
> + * pp_handle_indent() prints out the intendation, and
> + * perhaps the whole line (without the final newline)
> + *
> + * Why "perhaps"? If there are tabs in the indented line
> + * it will print it out in order to de-tabify the line.
> + *
> + * But if there are no tabs, we just fall back on the
> + * normal "print the whole line".
> + */
> +static int pp_handle_indent(struct strbuf *sb, int indent,
> +			     const char *line, int linelen)
> +{
> +	const char *tab;
> +
> +	strbuf_addchars(sb, ' ', indent);
> +
> +	tab = memchr(line, '\t', linelen);
> +	if (!tab)
> +		return 0;
> +
> +	do {
> +		int width = pp_utf8_width(line, tab);
> +
> +		/*
> +		 * If it wasn't well-formed utf8, or it
> +		 * had characters with badly defined
> +		 * width (control characters etc), just
> +		 * give up on trying to align things.
> +		 */
> +		if (width < 0)
> +			break;
> +
> +		/* Output the data .. */
> +		strbuf_add(sb, line, tab - line);
> +
> +		/* .. and the de-tabified tab */
> +		strbuf_addchars(sb, ' ', 8-(width & 7));
> +
> +		/* Skip over the printed part .. */
> +		linelen -= 1+tab-line;
> +		line = tab + 1;
> +
> +		/* .. and look for the next tab */
> +		tab = memchr(line, '\t', linelen);
> +	} while (tab);
> +
> +	/*
> +	 * Print out everything after the last tab without
> +	 * worrying about width - there's nothing more to
> +	 * align.
> +	 */
> +	strbuf_add(sb, line, linelen);
> +	return 1;
> +}
> +
>  void pp_remainder(struct pretty_print_context *pp,
>  		  const char **msg_p,
>  		  struct strbuf *sb,
> @@ -1652,8 +1722,10 @@ void pp_remainder(struct pretty_print_context *pp,
>  		first = 0;
>  
>  		strbuf_grow(sb, linelen + indent + 20);
> -		if (indent)
> -			strbuf_addchars(sb, ' ', indent);
> +		if (indent) {
> +			if (pp_handle_indent(sb, indent, line, linelen))
> +				linelen = 0;
> +		}
>  		strbuf_add(sb, line, linelen);
>  		strbuf_addch(sb, '\n');
>  	}

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Linus Torvalds]

On Wed, Mar 16, 2016 at 11:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>  (1) if turning your "preparation; do { ... } while()" into
>      "while () { }" would make the result a bit easier to read;

So it's probably partly taste, but I will also disagree with your
"easier to read", because of the way the code is logically structured.

In particular, the "no TAB" case is actually *fundamentally* different
from the "no TAB at the end" case. The return value is different, and
the caller does very different things - the code tries to make it very
clear that that "no TAB" situation is very different from "we found a
TAB".

So it's not "preparation + do-while".

It's "preparation + handle the no-TAB case differently", and then the
"do-while" is very natural because by the time we get to the "ok, we
are now going to need to do something about the line" stage, we
already know we have a tab.

But the code *could* be made to just always do the whole
"strbuf_add()", and not return a return value at all, and the no-tab
case wouldn't be explicitly written to be different.

Let me know if you'd prefer that variant, and I'll send a new version.

>  (2) if we can somehow eliminate duplication of "tab + 1" (spelled
>      differently on the previous line as "1+tab"), the end result
>      may get easier to follow.

Yeah, I considered that. Either by just doing "tab++" before (so the
+1" would come from that in both cases), or by introducing a new
variable like

    ptrdiff_t bytes_used;
    ...
    bytes_used = 1 + tab - line;

and then just doing

    line += bytes_used;
    linelen -= bytes_used;

and the code I wrote just didn't do any of those temporary updates,
and instead just did the "+1" by hand in both cases.

Again, I can redo the patch, just tell me which model you prefer.

                 Linus

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, Mar 16, 2016 at 11:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>  (1) if turning your "preparation; do { ... } while()" into
>>      "while () { }" would make the result a bit easier to read;
>
> So it's probably partly taste, but I will also disagree with your
> "easier to read", because of the way the code is logically structured.
>
> In particular, the "no TAB" case is actually *fundamentally* different
> from the "no TAB at the end" case. The return value is different, and
> the caller does very different things - the code tries to make it very
> clear that that "no TAB" situation is very different from "we found a
> TAB".
>
> So it's not "preparation + do-while".
>
> It's "preparation + handle the no-TAB case differently", and then the
> "do-while" is very natural because by the time we get to the "ok, we
> are now going to need to do something about the line" stage, we
> already know we have a tab.

OK, I agree with that viewpoint; retracted.

>>  (2) if we can somehow eliminate duplication of "tab + 1" (spelled
>>      differently on the previous line as "1+tab"), the end result
>>      may get easier to follow.
>
> Yeah, I considered that. Either by just doing "tab++" before (so the
> +1" would come from that in both cases), or by introducing a new
> variable like
>
>     ptrdiff_t bytes_used;
>     ...
>     bytes_used = 1 + tab - line;
>
> and then just doing
>
>     line += bytes_used;
>     linelen -= bytes_used;
>
> and the code I wrote just didn't do any of those temporary updates,
> and instead just did the "+1" by hand in both cases.

The above is most likely what I would have written if I were doing
this patch.  I could squash it to save a round-trip, but let me run
the testsuite first to see if we need adjustments to existing tests.

Also your idea:

> But the code *could* be made to just always do the whole
> "strbuf_add()", and not return a return value at all, and the no-tab
> case wouldn't be explicitly written to be different.

may give us a better structure if we are going to give users a knob
to disable this tab expansion, i.e. move the addition of 4 spaces to
the caller, name the body of such a function strbuf_expand_add(),
and then make the caller do something like this perhaps?

@@ -1723,10 +1711,14 @@ void pp_remainder(struct pretty_print_context *pp,
 
 		strbuf_grow(sb, linelen + indent + 20);
 		if (indent) {
-			if (pp_handle_indent(sb, indent, line, linelen))
-				linelen = 0;
+			strbuf_addchars(sb, ' ', indent);
+			if (pp->fmt == CMIT_FMT_EXPAND_TABS)
+				strbuf_expand_add(sb, line, linelen);
+			else
+				strbuf_add(sb, line, linelen);
+		} else {
+			strbuf_add(sb, line, linelen);
 		}
-		strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
 	}
 }

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> The above is most likely what I would have written if I were doing
> this patch.  I could squash it to save a round-trip, but let me run
> the testsuite first to see if we need adjustments to existing tests.

Strangely running t4201 with your patch (without any squashing)
seems to show a breakage in shortlog.  I won't be able to come back
to this topic for at least a few hours, so this is just a single bit
"breaks" report, without "how and why" analysis, sorry.

>
> Also your idea:
>
>> But the code *could* be made to just always do the whole
>> "strbuf_add()", and not return a return value at all, and the no-tab
>> case wouldn't be explicitly written to be different.
>
> may give us a better structure if we are going to give users a knob
> to disable this tab expansion, i.e. move the addition of 4 spaces to
> the caller, name the body of such a function strbuf_expand_add(),
> and then make the caller do something like this perhaps?
>
> @@ -1723,10 +1711,14 @@ void pp_remainder(struct pretty_print_context *pp,
>  
>  		strbuf_grow(sb, linelen + indent + 20);
>  		if (indent) {
> -			if (pp_handle_indent(sb, indent, line, linelen))
> -				linelen = 0;
> +			strbuf_addchars(sb, ' ', indent);
> +			if (pp->fmt == CMIT_FMT_EXPAND_TABS)
> +				strbuf_expand_add(sb, line, linelen);
> +			else
> +				strbuf_add(sb, line, linelen);
> +		} else {
> +			strbuf_add(sb, line, linelen);
>  		}
> -		strbuf_add(sb, line, linelen);
>  		strbuf_addch(sb, '\n');
>  	}
>  }

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]

On Wed, Mar 16, 2016 at 12:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> may give us a better structure if we are going to give users a knob
> to disable this tab expansion, i.e. move the addition of 4 spaces to
> the caller, name the body of such a function strbuf_expand_add(),
> and then make the caller do something like this perhaps?

I'd suggest just putting that knob into the "pp_handle_indent()"
function, and passing it the "pp" pointer.

In fact, maybe it should just be renamed as "pp_add_line()", and
handle every case, and keep "pp_remainder()" as just the "loop over
each line and handle the empty line and PP_SHORT special case" thing.

That makes it easty to add that CMIT_FMT_EXPAND_TABS kind of code later.

Here's an incremental patch that could be just smushed into my
previous one. It doesn't change the behavior of "pp_handle_indent()",
but I think it clarifies the code and makes future changes much easier
(partly because now nobody has to worry about the continue case and
the newline at the end of the line, so you can just print whatever you
want and then return).

What do you think?

                 Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 1030 bytes --]

 pretty.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/pretty.c b/pretty.c
index 0b40457f99f0..b9374a1708d1 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1699,6 +1699,18 @@ static int pp_handle_indent(struct strbuf *sb, int indent,
 	return 1;
 }
 
+static void pp_add_line(struct pretty_print_context *pp,
+			struct strbuf *sb, int indent,
+			const char *line, int linelen)
+{
+	strbuf_grow(sb, linelen + indent + 20);
+	if (indent) {
+		if (pp_handle_indent(sb, indent, line, linelen))
+			return;
+	}
+	strbuf_add(sb, line, linelen);
+}
+
 void pp_remainder(struct pretty_print_context *pp,
 		  const char **msg_p,
 		  struct strbuf *sb,
@@ -1721,12 +1733,7 @@ void pp_remainder(struct pretty_print_context *pp,
 		}
 		first = 0;
 
-		strbuf_grow(sb, linelen + indent + 20);
-		if (indent) {
-			if (pp_handle_indent(sb, indent, line, linelen))
-				linelen = 0;
-		}
-		strbuf_add(sb, line, linelen);
+		pp_add_line(pp, sb, indent, line, linelen);
 		strbuf_addch(sb, '\n');
 	}
 }

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Linus Torvalds]

On Wed, Mar 16, 2016 at 12:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Strangely running t4201 with your patch (without any squashing)
> seems to show a breakage in shortlog.  I won't be able to come back
> to this topic for at least a few hours, so this is just a single bit
> "breaks" report, without "how and why" analysis, sorry.

It's because those things have tabs in their first line, so the output
now differs from the expected one exactly because of the tab-vs-space
expansion.

The wrapping logic is then also different, because the .wrapping code
does the tabs as "align to 8 chars" while the new code does tabs as
"align to 8 chars modulo the indent offset".

I only looked at the first case, but I assume the others are just more
of the same. We'd just adjust the expected output, I assume.

               Linus

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, Mar 16, 2016 at 12:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Strangely running t4201 with your patch (without any squashing)
>> seems to show a breakage in shortlog.  I won't be able to come back
>> to this topic for at least a few hours, so this is just a single bit
>> "breaks" report, without "how and why" analysis, sorry.
>
> It's because those things have tabs in their first line, so the output
> now differs from the expected one exactly because of the tab-vs-space
> expansion.

What surprised me was that this new expand logic triggered for
shortlog, actually.  I somehow assumed the caller that called
de-tabify helper was only called for --pretty=medium.

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Here's an incremental patch that could be just smushed into my
> previous one. It doesn't change the behavior of "pp_handle_indent()",
> but I think it clarifies the code and makes future changes much easier
> (partly because now nobody has to worry about the continue case and
> the newline at the end of the line, so you can just print whatever you
> want and then return).
>
> What do you think?

I actually was hoping that strbuf_tabexpand_add() would be an
independently useful addition to strbuf_add*() family that does not
have to know the indentation and pretty-print-context, so burying
that new logic deep in the callchain like the patch below, while
letting it still be aware of "indent" value (and adding the leading
indent) does not look like a good abstraction from that point of
view.

The change to pp_remainder() looks like a nice cleanup for people
who read that function.  I still think pp_add_line() should add the
indentation to sb in "if (indent)" block itself, and the called
helper should be a "I know how to tab-expand a string and add the
result to a strbuf" that we can eventually move to strbuf.[ch].

Things like adding CMIT_FMT_EXPAND_TABS can still then be done with
something like this:

	strbuf_grow();
	if (indent) {
		strbuf_addchars(sb, ' ', indent);
-        	strbuf_tabexpand_add();
+		if (pp->format == EXPAND_TABS)
+	        	strbuf_tabexpand_add();
+		else
+	        	strbuf_add();
	} else {
        	strbuf_add();
	}

in pp_add_line(), so there is no difference between the ease of
future changes, I'd think.

>                  Linus
>
>  pretty.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index 0b40457f99f0..b9374a1708d1 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1699,6 +1699,18 @@ static int pp_handle_indent(struct strbuf *sb, int indent,
>  	return 1;
>  }
>  
> +static void pp_add_line(struct pretty_print_context *pp,
> +			struct strbuf *sb, int indent,
> +			const char *line, int linelen)
> +{
> +	strbuf_grow(sb, linelen + indent + 20);
> +	if (indent) {
> +		if (pp_handle_indent(sb, indent, line, linelen))
> +			return;
> +	}
> +	strbuf_add(sb, line, linelen);
> +}
> +
>  void pp_remainder(struct pretty_print_context *pp,
>  		  const char **msg_p,
>  		  struct strbuf *sb,
> @@ -1721,12 +1733,7 @@ void pp_remainder(struct pretty_print_context *pp,
>  		}
>  		first = 0;
>  
> -		strbuf_grow(sb, linelen + indent + 20);
> -		if (indent) {
> -			if (pp_handle_indent(sb, indent, line, linelen))
> -				linelen = 0;
> -		}
> -		strbuf_add(sb, line, linelen);
> +		pp_add_line(pp, sb, indent, line, linelen);
>  		strbuf_addch(sb, '\n');
>  	}
>  }

Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly

[Linus Torvalds]

On Wed, Mar 16, 2016 at 2:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> What surprised me was that this new expand logic triggered for
> shortlog, actually.  I somehow assumed the caller that called
> de-tabify helper was only called for --pretty=medium.

I guess that would be ok, since shortlog by definition can't have any
issues with multiple lines lining up with each other.

At the same time, it might be a bit odd to show tabs in that first
line differently for the one-line vs multi-line log version. But maybe
it isn't - I think shortlog is the only thing that does that wrapping
anyway, so shortlog is already special.

I think the reason shortlog output gets both the de-tab and the
wrapping is that shortlog_add_commit() just calls pretty_print_commit
with CMIT_FMT_USERFORMAT.

            Linus

[PATCH v2 1/4] pretty-print: de-tabify indented logs to make things line up properly

[Junio C Hamano]

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 16 Mar 2016 09:15:53 -0700

A commit log message sometimes tries to line things up using tabs,
assuming fixed-width font with the standard 8-place tab settings.
Viewing such a commit however does not work well in "git log", as we
indent the lines by prefixing 4 spaces in front of them.

This should all line up:

  Column 1	Column 2
  --------	--------
  A		B
  ABCD		EFGH
  SPACES        Instead of Tabs

Even with multi-byte UTF8 characters:

  Column 1	Column 2
  --------	--------
  Ä		B
  åäö		100
  A Møøse	once bit my sister..

Tab-expand the lines in "git log --pretty=medium" output (which is
the default), before prefixing 4 spaces.

This breaks a few tests in t4201, that tests "git shortlog".

 - One passes "git log" output to "git shortlog" to use the latter
   as a filter and does not expect the output of the former to be
   de-tabified.

 - The other expects that "git shortlog", when it reads the first
   line of the commit and produces the output itself, does not
   de-tabify it.

Mark them as expecting failure for now.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 pretty.c            | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 t/t4201-shortlog.sh |  4 +--
 2 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/pretty.c b/pretty.c
index 92b2870..0b40457 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1629,6 +1629,76 @@ void pp_title_line(struct pretty_print_context *pp,
 	strbuf_release(&title);
 }
 
+static int pp_utf8_width(const char *start, const char *end)
+{
+	int width = 0;
+	size_t remain = end - start;
+
+	while (remain) {
+		int n = utf8_width(&start, &remain);
+		if (n < 0 || !start)
+			return -1;
+		width += n;
+	}
+	return width;
+}
+
+/*
+ * pp_handle_indent() prints out the intendation, and
+ * perhaps the whole line (without the final newline)
+ *
+ * Why "perhaps"? If there are tabs in the indented line
+ * it will print it out in order to de-tabify the line.
+ *
+ * But if there are no tabs, we just fall back on the
+ * normal "print the whole line".
+ */
+static int pp_handle_indent(struct strbuf *sb, int indent,
+			     const char *line, int linelen)
+{
+	const char *tab;
+
+	strbuf_addchars(sb, ' ', indent);
+
+	tab = memchr(line, '\t', linelen);
+	if (!tab)
+		return 0;
+
+	do {
+		int width = pp_utf8_width(line, tab);
+
+		/*
+		 * If it wasn't well-formed utf8, or it
+		 * had characters with badly defined
+		 * width (control characters etc), just
+		 * give up on trying to align things.
+		 */
+		if (width < 0)
+			break;
+
+		/* Output the data .. */
+		strbuf_add(sb, line, tab - line);
+
+		/* .. and the de-tabified tab */
+		strbuf_addchars(sb, ' ', 8-(width & 7));
+
+		/* Skip over the printed part .. */
+		linelen -= 1+tab-line;
+		line = tab + 1;
+
+		/* .. and look for the next tab */
+		tab = memchr(line, '\t', linelen);
+	} while (tab);
+
+	/*
+	 * Print out everything after the last tab without
+	 * worrying about width - there's nothing more to
+	 * align.
+	 */
+	strbuf_add(sb, line, linelen);
+	return 1;
+}
+
 void pp_remainder(struct pretty_print_context *pp,
 		  const char **msg_p,
 		  struct strbuf *sb,
@@ -1652,8 +1722,10 @@ void pp_remainder(struct pretty_print_context *pp,
 		first = 0;
 
 		strbuf_grow(sb, linelen + indent + 20);
-		if (indent)
-			strbuf_addchars(sb, ' ', indent);
+		if (indent) {
+			if (pp_handle_indent(sb, indent, line, linelen))
+				linelen = 0;
+		}
 		strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
 	}
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 7600a3e..987b708 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_success !MINGW 'shortlog wrapping' '
+test_expect_failure !MINGW 'shortlog wrapping' '
 	cat >expect <<\EOF &&
 A U Thor (5):
       Test
@@ -114,7 +114,7 @@ EOF
 	test_cmp expect out
 '
 
-test_expect_success !MINGW 'shortlog from non-git directory' '
+test_expect_failure !MINGW 'shortlog from non-git directory' '
 	git log HEAD >log &&
 	GIT_DIR=non-existing git shortlog -w <log >out &&
 	test_cmp expect out
-- 
2.8.0-rc3-175-g64dcf62

[PATCH v2 2/4] pretty-print: simplify the interaction between pp_handle_indent() and its caller

[Junio C Hamano]

Instead of	sometimes handling the output itself and some other
times forcing   the caller handle the output, make pp_handle_indent()
responsible to  handle the output for all cases.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This and the other two patches that follow show what I meant
   during the discussion.

 pretty.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/pretty.c b/pretty.c
index 0b40457..6d657fc 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1645,26 +1645,17 @@ static int pp_utf8_width(const char *start, const char *end)
 
 /*
  * pp_handle_indent() prints out the intendation, and
- * perhaps the whole line (without the final newline)
- *
- * Why "perhaps"? If there are tabs in the indented line
- * it will print it out in order to de-tabify the line.
- *
- * But if there are no tabs, we just fall back on the
- * normal "print the whole line".
+ * the whole line (without the final newline), after
+ * de-tabifying.
  */
-static int pp_handle_indent(struct strbuf *sb, int indent,
+static void pp_handle_indent(struct strbuf *sb, int indent,
 			     const char *line, int linelen)
 {
 	const char *tab;
 
 	strbuf_addchars(sb, ' ', indent);
 
-	tab = memchr(line, '\t', linelen);
-	if (!tab)
-		return 0;
-
-	do {
+	while ((tab = memchr(line, '\t', linelen)) != NULL) {
 		int width = pp_utf8_width(line, tab);
 
 		/*
@@ -1685,10 +1676,7 @@ static int pp_handle_indent(struct strbuf *sb, int indent,
 		/* Skip over the printed part .. */
 		linelen -= 1+tab-line;
 		line = tab + 1;
-
-		/* .. and look for the next tab */
-		tab = memchr(line, '\t', linelen);
-	} while (tab);
+	}
 
 	/*
 	 * Print out everything after the last tab without
@@ -1696,7 +1684,6 @@ static int pp_handle_indent(struct strbuf *sb, int indent,
 	 * align.
 	 */
 	strbuf_add(sb, line, linelen);
-	return 1;
 }
 
 void pp_remainder(struct pretty_print_context *pp,
@@ -1722,11 +1709,10 @@ void pp_remainder(struct pretty_print_context *pp,
 		first = 0;
 
 		strbuf_grow(sb, linelen + indent + 20);
-		if (indent) {
-			if (pp_handle_indent(sb, indent, line, linelen))
-				linelen = 0;
-		}
-		strbuf_add(sb, line, linelen);
+		if (indent)
+			pp_handle_indent(sb, indent, line, linelen);
+		else
+			strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
 	}
 }
-- 
2.8.0-rc3-175-g64dcf62

[PATCH v2 3/4] pretty-print: further abstract out pp_handle_indent()

[Junio C Hamano]

Separate the call to add 4-space indent, and a new helper to add a
line after de-tabifying.

The new helper function strbuf_add_tabexpand() could later be moved
to strbuf.[ch] if other callers need to.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 pretty.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/pretty.c b/pretty.c
index 6d657fc..717ceed 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1643,18 +1643,12 @@ static int pp_utf8_width(const char *start, const char *end)
 	return width;
 }
 
-/*
- * pp_handle_indent() prints out the intendation, and
- * the whole line (without the final newline), after
- * de-tabifying.
- */
-static void pp_handle_indent(struct strbuf *sb, int indent,
-			     const char *line, int linelen)
+
+static void strbuf_add_tabexpand(struct strbuf *sb,
+				 const char *line, int linelen)
 {
 	const char *tab;
 
-	strbuf_addchars(sb, ' ', indent);
-
 	while ((tab = memchr(line, '\t', linelen)) != NULL) {
 		int width = pp_utf8_width(line, tab);
 
@@ -1686,6 +1680,18 @@ static void pp_handle_indent(struct strbuf *sb, int indent,
 	strbuf_add(sb, line, linelen);
 }
 
+/*
+ * pp_handle_indent() prints out the intendation, and
+ * the whole line (without the final newline), after
+ * de-tabifying.
+ */
+static void pp_handle_indent(struct strbuf *sb, int indent,
+			     const char *line, int linelen)
+{
+	strbuf_addchars(sb, ' ', indent);
+	strbuf_add_tabexpand(sb, line, linelen);
+}
+
 void pp_remainder(struct pretty_print_context *pp,
 		  const char **msg_p,
 		  struct strbuf *sb,
-- 
2.8.0-rc3-175-g64dcf62

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

[Junio C Hamano]

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 <gitster@pobox.com>
---
 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

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

[Linus Torvalds]

On Thu, Mar 17, 2016 at 4:16 PM, Junio C Hamano <gitster@pobox.com> 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

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

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Thu, Mar 17, 2016 at 4:16 PM, Junio C Hamano <gitster@pobox.com> 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.

Sorry is mutual; I would have done this much earlier if I didn't
have the four-maintenance-tracks-at-the-same-time release today ;-)

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

[Jeff King]

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

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

[Linus Torvalds]

On Thu, Mar 17, 2016 at 10:08 PM, Jeff King <peff@peff.net> 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

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> 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).

Yup, I like that better, but it is now past my work hours, so I
won't be looking at it right now.

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

[Jeff King]

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

> On Thu, Mar 17, 2016 at 10:08 PM, Jeff King <peff@peff.net> 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).

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

[PATCH v3 0/5] Expanding tabs in "git log" output

[Junio C Hamano]

So here is the third try (previous round is found at $gmane/289166
and the very first one is at $gmane/288987).

The first three patches are essentially the same as v2.  The last
two updates how the tab-expansion is internally controlled:

 [4/5] adds a bit to pretty-commit-context that tells if tabs should
       be expanded.  Unlike v2 that tied this to pretty print format,
       this bit is orthogonal to the format, and theoretically it is
       possible to expand tabs even with --format=email.  Also,
       unlike v2, tabs are expanded not just in `medium` format, but
       also in `full` and `fuller` formats.

 [5/5] adds a new option --no-expand-tabs that controls the bit 4/5
       introduces, so that "git log [--pretty] --no-expand-tabs"
       would show the log message indented by 4 spaces, without tab
       expansion.

By the way, I have to say that I hate how pretty formatting and
revision machinery interact with each other.

pretty.c::commit_formats ought to be the authoritative source of how
each named format should work, but there are quite a many codepaths
that just assign CMIT_FMT_SOMETHING to revs->commit_format without
bothering with other fields in the cmt_fmt_map like is_tformat, and
I am not sure if they are working correctly even before this patch.

Junio C Hamano (4):
  pretty-print: simplify the interaction between pp_handle_indent() and its caller
  pretty-print: further abstract out pp_handle_indent()
  pretty-print: limit expand-tabs to selected --pretty formats
  pretty-print: teach "--no-expand-tabs" option to "git log"

Linus Torvalds (1):
  pretty-print: de-tabify indented logs to make things line up properly

 Documentation/pretty-options.txt |  6 +++
 commit.h                         |  1 +
 log-tree.c                       |  1 +
 pretty.c                         | 88 ++++++++++++++++++++++++++++++++++++----
 revision.c                       |  3 ++
 revision.h                       |  1 +
 t/t4201-shortlog.sh              |  2 +-
 7 files changed, 92 insertions(+), 10 deletions(-)

[References]

http://thread.gmane.org/gmane.comp.version-control.git/288987/focus=289166


Interdiff since v2 is shown below.

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 173b932..671cebd 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -39,16 +39,6 @@ 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 7032b1a..069b927 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', 'noexpand', 'format:<string>'
+	'full', 'fuller', 'email', 'raw', '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.
@@ -42,6 +42,12 @@ people using 80-column terminals.
 	verbatim; this means that invalid sequences in the original
 	commit may be copied to the output.
 
+--no-expand-tabs::
+	The formats that indent the log message by 4 spaces
+	(i.e. 'medium', 'full', and 'fuller') by default show tabs
+	in the log message expanded.  This option disables the
+	expansion.
+
 ifndef::git-rev-list[]
 --notes[=<ref>]::
 	Show the notes (see linkgit:git-notes[1]) that annotate the
diff --git a/commit.h b/commit.h
index d511c61..a7ef682 100644
--- a/commit.h
+++ b/commit.h
@@ -126,7 +126,6 @@ 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,
@@ -148,6 +147,7 @@ struct pretty_print_context {
 	int preserve_subject;
 	struct date_mode date_mode;
 	unsigned date_mode_explicit:1;
+	unsigned expand_tabs_in_log:1;
 	int need_8bit_cte;
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
diff --git a/log-tree.c b/log-tree.c
index 60f9839..78a5381 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -683,6 +683,7 @@ void show_log(struct rev_info *opt)
 	ctx.fmt = opt->commit_format;
 	ctx.mailmap = opt->mailmap;
 	ctx.color = opt->diffopt.use_color;
+	ctx.expand_tabs_in_log = opt->expand_tabs_in_log;
 	ctx.output_encoding = get_log_output_encoding();
 	if (opt->from_ident.mail_begin && opt->from_ident.name_begin)
 		ctx.from_ident = &opt->from_ident;
diff --git a/pretty.c b/pretty.c
index 8b533dc..5a33b7e 100644
--- a/pretty.c
+++ b/pretty.c
@@ -16,6 +16,7 @@ static struct cmt_fmt_map {
 	const char *name;
 	enum cmit_fmt format;
 	int is_tformat;
+	int expand_tabs_in_log;
 	int is_alias;
 	const char *user_format;
 } *commit_formats;
@@ -87,14 +88,13 @@ static int git_pretty_formats_config(const char *var, const char *value, void *c
 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 },
-		{ "full",	CMIT_FMT_FULL,		0 },
-		{ "oneline",	CMIT_FMT_ONELINE,	1 }
+		{ "raw",	CMIT_FMT_RAW,		0,	0 },
+		{ "medium",	CMIT_FMT_MEDIUM,	0,	1 },
+		{ "short",	CMIT_FMT_SHORT,		0,	0 },
+		{ "email",	CMIT_FMT_EMAIL,		0,	0 },
+		{ "fuller",	CMIT_FMT_FULLER,	0,	1 },
+		{ "full",	CMIT_FMT_FULL,		0,	1 },
+		{ "oneline",	CMIT_FMT_ONELINE,	1,	0 }
 	};
 	commit_formats_len = ARRAY_SIZE(builtin_formats);
 	builtin_formats_len = commit_formats_len;
@@ -173,6 +173,7 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 
 	rev->commit_format = commit_format->format;
 	rev->use_terminator = commit_format->is_tformat;
+	rev->expand_tabs_in_log = commit_format->expand_tabs_in_log;
 	if (commit_format->format == CMIT_FMT_USERFORMAT) {
 		save_user_format(rev, commit_format->user_format,
 				 commit_format->is_tformat);
@@ -1687,12 +1688,11 @@ static void strbuf_add_tabexpand(struct strbuf *sb,
  * de-tabifying.
  */
 static void pp_handle_indent(struct pretty_print_context *pp,
-			     struct strbuf *sb,
-			     int indent,
+			     struct strbuf *sb, int indent,
 			     const char *line, int linelen)
 {
 	strbuf_addchars(sb, ' ', indent);
-	if (pp->fmt == CMIT_FMT_MEDIUM)
+	if (pp->expand_tabs_in_log)
 		strbuf_add_tabexpand(sb, line, linelen);
 	else
 		strbuf_add(sb, line, linelen);
diff --git a/revision.c b/revision.c
index df56fce..b0d2a36 100644
--- a/revision.c
+++ b/revision.c
@@ -1412,6 +1412,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->skip_count = -1;
 	revs->max_count = -1;
 	revs->max_parents = -1;
+	revs->expand_tabs_in_log = 1;
 
 	revs->commit_format = CMIT_FMT_DEFAULT;
 
@@ -1915,6 +1916,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->verbose_header = 1;
 		revs->pretty_given = 1;
 		get_commit_format(arg+9, revs);
+	} else if (!strcmp(arg, "--no-expand-tabs")) {
+		revs->expand_tabs_in_log = 0;
 	} else if (!strcmp(arg, "--show-notes") || !strcmp(arg, "--notes")) {
 		revs->show_notes = 1;
 		revs->show_notes_given = 1;
diff --git a/revision.h b/revision.h
index 23857c0..0bbfe0e 100644
--- a/revision.h
+++ b/revision.h
@@ -137,6 +137,7 @@ struct rev_info {
 			abbrev_commit_given:1,
 			zero_commit:1,
 			use_terminator:1,
+			expand_tabs_in_log:1,
 			missing_newline:1,
 			date_mode_explicit:1,
 			preserve_subject:1;
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 34a9fed..2fec948 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -115,7 +115,7 @@ EOF
 '
 
 test_expect_success !MINGW 'shortlog from non-git directory' '
-	git log --pretty=noexpand HEAD >log &&
+	git log --no-expand-tabs HEAD >log &&
 	GIT_DIR=non-existing git shortlog -w <log >out &&
 	test_cmp expect out
 '

[PATCH v3 1/5] pretty-print: de-tabify indented logs to make things line up properly

[Junio C Hamano]

From: Linus Torvalds <torvalds@linux-foundation.org>

A commit log message sometimes tries to line things up using tabs,
assuming fixed-width font with the standard 8-place tab settings.
Viewing such a commit however does not work well in "git log", as we
indent the lines by prefixing 4 spaces in front of them.

This should all line up:

  Column 1	Column 2
  --------	--------
  A		B
  ABCD		EFGH
  SPACES        Instead of Tabs

Even with multi-byte UTF8 characters:

  Column 1	Column 2
  --------	--------
  Ä		B
  åäö		100
  A Møøse	once bit my sister..

Tab-expand the lines in "git log --pretty=medium" output (which is
the default), before prefixing 4 spaces.

This breaks a few tests in t4201, that tests "git shortlog".

 - One passes "git log" output to "git shortlog" to use the latter
   as a filter and does not expect the output of the former to be
   de-tabified.

 - The other expects that "git shortlog", when it reads the first
   line of the commit and produces the output itself, does not
   de-tabify it.

Mark them as expecting failure for now.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 pretty.c            | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 t/t4201-shortlog.sh |  4 +--
 2 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/pretty.c b/pretty.c
index 92b2870..0b40457 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1629,6 +1629,76 @@ void pp_title_line(struct pretty_print_context *pp,
 	strbuf_release(&title);
 }
 
+static int pp_utf8_width(const char *start, const char *end)
+{
+	int width = 0;
+	size_t remain = end - start;
+
+	while (remain) {
+		int n = utf8_width(&start, &remain);
+		if (n < 0 || !start)
+			return -1;
+		width += n;
+	}
+	return width;
+}
+
+/*
+ * pp_handle_indent() prints out the intendation, and
+ * perhaps the whole line (without the final newline)
+ *
+ * Why "perhaps"? If there are tabs in the indented line
+ * it will print it out in order to de-tabify the line.
+ *
+ * But if there are no tabs, we just fall back on the
+ * normal "print the whole line".
+ */
+static int pp_handle_indent(struct strbuf *sb, int indent,
+			     const char *line, int linelen)
+{
+	const char *tab;
+
+	strbuf_addchars(sb, ' ', indent);
+
+	tab = memchr(line, '\t', linelen);
+	if (!tab)
+		return 0;
+
+	do {
+		int width = pp_utf8_width(line, tab);
+
+		/*
+		 * If it wasn't well-formed utf8, or it
+		 * had characters with badly defined
+		 * width (control characters etc), just
+		 * give up on trying to align things.
+		 */
+		if (width < 0)
+			break;
+
+		/* Output the data .. */
+		strbuf_add(sb, line, tab - line);
+
+		/* .. and the de-tabified tab */
+		strbuf_addchars(sb, ' ', 8-(width & 7));
+
+		/* Skip over the printed part .. */
+		linelen -= 1+tab-line;
+		line = tab + 1;
+
+		/* .. and look for the next tab */
+		tab = memchr(line, '\t', linelen);
+	} while (tab);
+
+	/*
+	 * Print out everything after the last tab without
+	 * worrying about width - there's nothing more to
+	 * align.
+	 */
+	strbuf_add(sb, line, linelen);
+	return 1;
+}
+
 void pp_remainder(struct pretty_print_context *pp,
 		  const char **msg_p,
 		  struct strbuf *sb,
@@ -1652,8 +1722,10 @@ void pp_remainder(struct pretty_print_context *pp,
 		first = 0;
 
 		strbuf_grow(sb, linelen + indent + 20);
-		if (indent)
-			strbuf_addchars(sb, ' ', indent);
+		if (indent) {
+			if (pp_handle_indent(sb, indent, line, linelen))
+				linelen = 0;
+		}
 		strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
 	}
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 7600a3e..987b708 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_success !MINGW 'shortlog wrapping' '
+test_expect_failure !MINGW 'shortlog wrapping' '
 	cat >expect <<\EOF &&
 A U Thor (5):
       Test
@@ -114,7 +114,7 @@ EOF
 	test_cmp expect out
 '
 
-test_expect_success !MINGW 'shortlog from non-git directory' '
+test_expect_failure !MINGW 'shortlog from non-git directory' '
 	git log HEAD >log &&
 	GIT_DIR=non-existing git shortlog -w <log >out &&
 	test_cmp expect out
-- 
2.8.0-rc4-198-g3f6b64c

[PATCH v3 2/5] pretty-print: simplify the interaction between pp_handle_indent() and its caller

[Junio C Hamano]

Instead	of sometimes handling the output itself and some other times
forcing the caller handle the output, make the helper function
pp_handle_indent() responsible for the output for all cases.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 pretty.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/pretty.c b/pretty.c
index 0b40457..6d657fc 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1645,26 +1645,17 @@ static int pp_utf8_width(const char *start, const char *end)
 
 /*
  * pp_handle_indent() prints out the intendation, and
- * perhaps the whole line (without the final newline)
- *
- * Why "perhaps"? If there are tabs in the indented line
- * it will print it out in order to de-tabify the line.
- *
- * But if there are no tabs, we just fall back on the
- * normal "print the whole line".
+ * the whole line (without the final newline), after
+ * de-tabifying.
  */
-static int pp_handle_indent(struct strbuf *sb, int indent,
+static void pp_handle_indent(struct strbuf *sb, int indent,
 			     const char *line, int linelen)
 {
 	const char *tab;
 
 	strbuf_addchars(sb, ' ', indent);
 
-	tab = memchr(line, '\t', linelen);
-	if (!tab)
-		return 0;
-
-	do {
+	while ((tab = memchr(line, '\t', linelen)) != NULL) {
 		int width = pp_utf8_width(line, tab);
 
 		/*
@@ -1685,10 +1676,7 @@ static int pp_handle_indent(struct strbuf *sb, int indent,
 		/* Skip over the printed part .. */
 		linelen -= 1+tab-line;
 		line = tab + 1;
-
-		/* .. and look for the next tab */
-		tab = memchr(line, '\t', linelen);
-	} while (tab);
+	}
 
 	/*
 	 * Print out everything after the last tab without
@@ -1696,7 +1684,6 @@ static int pp_handle_indent(struct strbuf *sb, int indent,
 	 * align.
 	 */
 	strbuf_add(sb, line, linelen);
-	return 1;
 }
 
 void pp_remainder(struct pretty_print_context *pp,
@@ -1722,11 +1709,10 @@ void pp_remainder(struct pretty_print_context *pp,
 		first = 0;
 
 		strbuf_grow(sb, linelen + indent + 20);
-		if (indent) {
-			if (pp_handle_indent(sb, indent, line, linelen))
-				linelen = 0;
-		}
-		strbuf_add(sb, line, linelen);
+		if (indent)
+			pp_handle_indent(sb, indent, line, linelen);
+		else
+			strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
 	}
 }
-- 
2.8.0-rc4-198-g3f6b64c

[PATCH v3 3/5] pretty-print: further abstract out pp_handle_indent()

[Junio C Hamano]

Separate the call to add 4-space indent, and a new helper to add a
line after de-tabifying.

The new helper function strbuf_add_tabexpand() could later be moved
to strbuf.[ch] if other callers need to.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 pretty.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/pretty.c b/pretty.c
index 6d657fc..717ceed 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1643,18 +1643,12 @@ static int pp_utf8_width(const char *start, const char *end)
 	return width;
 }
 
-/*
- * pp_handle_indent() prints out the intendation, and
- * the whole line (without the final newline), after
- * de-tabifying.
- */
-static void pp_handle_indent(struct strbuf *sb, int indent,
-			     const char *line, int linelen)
+
+static void strbuf_add_tabexpand(struct strbuf *sb,
+				 const char *line, int linelen)
 {
 	const char *tab;
 
-	strbuf_addchars(sb, ' ', indent);
-
 	while ((tab = memchr(line, '\t', linelen)) != NULL) {
 		int width = pp_utf8_width(line, tab);
 
@@ -1686,6 +1680,18 @@ static void pp_handle_indent(struct strbuf *sb, int indent,
 	strbuf_add(sb, line, linelen);
 }
 
+/*
+ * pp_handle_indent() prints out the intendation, and
+ * the whole line (without the final newline), after
+ * de-tabifying.
+ */
+static void pp_handle_indent(struct strbuf *sb, int indent,
+			     const char *line, int linelen)
+{
+	strbuf_addchars(sb, ' ', indent);
+	strbuf_add_tabexpand(sb, line, linelen);
+}
+
 void pp_remainder(struct pretty_print_context *pp,
 		  const char **msg_p,
 		  struct strbuf *sb,
-- 
2.8.0-rc4-198-g3f6b64c

[PATCH v3 4/5] pretty-print: limit expand-tabs to selected --pretty formats

[Junio C Hamano]

Make sure that "git log" (by default, it uses --pretty=medium)
and "git log --pretty={full,fuller}" are the only ones that trigger
the new "expand tabs in the log message" behaviour.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 commit.h            |  1 +
 log-tree.c          |  1 +
 pretty.c            | 26 ++++++++++++++++----------
 revision.c          |  1 +
 revision.h          |  1 +
 t/t4201-shortlog.sh |  2 +-
 6 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/commit.h b/commit.h
index 5d58be0..a7ef682 100644
--- a/commit.h
+++ b/commit.h
@@ -147,6 +147,7 @@ struct pretty_print_context {
 	int preserve_subject;
 	struct date_mode date_mode;
 	unsigned date_mode_explicit:1;
+	unsigned expand_tabs_in_log:1;
 	int need_8bit_cte;
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
diff --git a/log-tree.c b/log-tree.c
index 60f9839..78a5381 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -683,6 +683,7 @@ void show_log(struct rev_info *opt)
 	ctx.fmt = opt->commit_format;
 	ctx.mailmap = opt->mailmap;
 	ctx.color = opt->diffopt.use_color;
+	ctx.expand_tabs_in_log = opt->expand_tabs_in_log;
 	ctx.output_encoding = get_log_output_encoding();
 	if (opt->from_ident.mail_begin && opt->from_ident.name_begin)
 		ctx.from_ident = &opt->from_ident;
diff --git a/pretty.c b/pretty.c
index 717ceed..5a33b7e 100644
--- a/pretty.c
+++ b/pretty.c
@@ -16,6 +16,7 @@ static struct cmt_fmt_map {
 	const char *name;
 	enum cmit_fmt format;
 	int is_tformat;
+	int expand_tabs_in_log;
 	int is_alias;
 	const char *user_format;
 } *commit_formats;
@@ -87,13 +88,13 @@ static int git_pretty_formats_config(const char *var, const char *value, void *c
 static void setup_commit_formats(void)
 {
 	struct cmt_fmt_map builtin_formats[] = {
-		{ "raw",	CMIT_FMT_RAW,		0 },
-		{ "medium",	CMIT_FMT_MEDIUM,	0 },
-		{ "short",	CMIT_FMT_SHORT,		0 },
-		{ "email",	CMIT_FMT_EMAIL,		0 },
-		{ "fuller",	CMIT_FMT_FULLER,	0 },
-		{ "full",	CMIT_FMT_FULL,		0 },
-		{ "oneline",	CMIT_FMT_ONELINE,	1 }
+		{ "raw",	CMIT_FMT_RAW,		0,	0 },
+		{ "medium",	CMIT_FMT_MEDIUM,	0,	1 },
+		{ "short",	CMIT_FMT_SHORT,		0,	0 },
+		{ "email",	CMIT_FMT_EMAIL,		0,	0 },
+		{ "fuller",	CMIT_FMT_FULLER,	0,	1 },
+		{ "full",	CMIT_FMT_FULL,		0,	1 },
+		{ "oneline",	CMIT_FMT_ONELINE,	1,	0 }
 	};
 	commit_formats_len = ARRAY_SIZE(builtin_formats);
 	builtin_formats_len = commit_formats_len;
@@ -172,6 +173,7 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 
 	rev->commit_format = commit_format->format;
 	rev->use_terminator = commit_format->is_tformat;
+	rev->expand_tabs_in_log = commit_format->expand_tabs_in_log;
 	if (commit_format->format == CMIT_FMT_USERFORMAT) {
 		save_user_format(rev, commit_format->user_format,
 				 commit_format->is_tformat);
@@ -1685,11 +1687,15 @@ 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->expand_tabs_in_log)
+		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/revision.c b/revision.c
index df56fce..8827d9f 100644
--- a/revision.c
+++ b/revision.c
@@ -1412,6 +1412,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->skip_count = -1;
 	revs->max_count = -1;
 	revs->max_parents = -1;
+	revs->expand_tabs_in_log = 1;
 
 	revs->commit_format = CMIT_FMT_DEFAULT;
 
diff --git a/revision.h b/revision.h
index 23857c0..0bbfe0e 100644
--- a/revision.h
+++ b/revision.h
@@ -137,6 +137,7 @@ struct rev_info {
 			abbrev_commit_given:1,
 			zero_commit:1,
 			use_terminator:1,
+			expand_tabs_in_log:1,
 			missing_newline:1,
 			date_mode_explicit:1,
 			preserve_subject:1;
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 987b708..d1e8259 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
-- 
2.8.0-rc4-198-g3f6b64c

[PATCH v3 5/5] pretty-print: teach "--no-expand-tabs" option to "git log"

[Junio C Hamano]

The output formats of "git log" that indent the log message by 4
spaces have been updated to expand tabs by default in previous
steps, without a way to restore the original behaviour.

Introduce a new "--no-expand-tabs" option to allow this.

As the effect of options is cumulative,

    $ git log [--pretty=medium] --no-expand-tabs

would not expand, while this invocation

    $ git log --no-expand-tabs --pretty[=medium]

by virtue of having --pretty later on the command line, expands tabs
again.

We _could_ introduce --expand-tabs option as well, to allow

    $ git log --pretty=email --expand-tabs

but we don't bother, as the output format that do not expand tabs by
default are mostly meant to transfer the contents as literally as
possible.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
---
 Documentation/pretty-options.txt | 6 ++++++
 revision.c                       | 2 ++
 t/t4201-shortlog.sh              | 4 ++--
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 4b659ac..069b927 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -42,6 +42,12 @@ people using 80-column terminals.
 	verbatim; this means that invalid sequences in the original
 	commit may be copied to the output.
 
+--no-expand-tabs::
+	The formats that indent the log message by 4 spaces
+	(i.e. 'medium', 'full', and 'fuller') by default show tabs
+	in the log message expanded.  This option disables the
+	expansion.
+
 ifndef::git-rev-list[]
 --notes[=<ref>]::
 	Show the notes (see linkgit:git-notes[1]) that annotate the
diff --git a/revision.c b/revision.c
index 8827d9f..b0d2a36 100644
--- a/revision.c
+++ b/revision.c
@@ -1916,6 +1916,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->verbose_header = 1;
 		revs->pretty_given = 1;
 		get_commit_format(arg+9, revs);
+	} else if (!strcmp(arg, "--no-expand-tabs")) {
+		revs->expand_tabs_in_log = 0;
 	} else if (!strcmp(arg, "--show-notes") || !strcmp(arg, "--notes")) {
 		revs->show_notes = 1;
 		revs->show_notes_given = 1;
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index d1e8259..2fec948 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -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 --no-expand-tabs HEAD >log &&
 	GIT_DIR=non-existing git shortlog -w <log >out &&
 	test_cmp expect out
 '
-- 
2.8.0-rc4-198-g3f6b64c

Re: [PATCH v3 0/5] Expanding tabs in "git log" output

[Linus Torvalds]

On Wed, Mar 23, 2016 at 4:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
> So here is the third try (previous round is found at $gmane/289166
> and the very first one is at $gmane/288987).
>
> The first three patches are essentially the same as v2.  The last
> two updates how the tab-expansion is internally controlled:

I tested this (as it in in 'pu', rather than applying the patches),
and it all seems to work fine. So Ack.

And I agree that it would be good if all the commit printout logic was
unified rather than having some ad-hoc "let's just set the format",
but I think that's a separate cleanup.

It might be more regular to have that "--expand-tabs" flag too (which
would then work with the email and raw formats), but I don't see any
actual real use for it so it really doesn't matter.

          Linus

Re: [PATCH v3 0/5] Expanding tabs in "git log" output

[Jeff King]

On Wed, Mar 23, 2016 at 04:23:41PM -0700, Junio C Hamano wrote:

> So here is the third try (previous round is found at $gmane/289166
> and the very first one is at $gmane/288987).

Is the plan to merge these as-is? The ordering is a bit funny (introduce
breakage, then repair it), and I think the first patch still breaks
t4201.8 (which is then repaired in the fourth one).

I think it would be a lot easier to review as:

  1. Factor out pp_handle_indent(), and any other preparation.

  2. Add --expand-tabs / --no-expand-tabs, with the logic going into
     pp_handle_indent().

  3. Flip the default for some formats to expand-tabs.

Other than that, the end result seems OK to me (I think adding
--expand-tabs would be nice, but I suspect it may need to be marked as
incompatible with some formats; do all formats end up in this same
writing code path?).

> By the way, I have to say that I hate how pretty formatting and
> revision machinery interact with each other.
> 
> pretty.c::commit_formats ought to be the authoritative source of how
> each named format should work, but there are quite a many codepaths
> that just assign CMIT_FMT_SOMETHING to revs->commit_format without
> bothering with other fields in the cmt_fmt_map like is_tformat, and
> I am not sure if they are working correctly even before this patch.

I don't disagree with any of that. I suspect some of the logic may be
complicated for sticking in a table, though. Perhaps we need a:

  void set_pp_format(struct pretty_print_context *ctx, enum cmit_fmt fmt);

that sets up the whole struct based on the given format, and then the
logic can live in C code. I haven't looked closely at that code in a
while, though, so maybe that is overkill.

-Peff

Re: [PATCH v3 0/5] Expanding tabs in "git log" output

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Mar 23, 2016 at 04:23:41PM -0700, Junio C Hamano wrote:
>
>> So here is the third try (previous round is found at $gmane/289166
>> and the very first one is at $gmane/288987).
>
> Is the plan to merge these as-is? The ordering is a bit funny (introduce
> breakage, then repair it), and I think the first patch still breaks
> t4201.8 (which is then repaired in the fourth one).

I do not have a firm plan yet.  This was one of those "during the
pre-release freeze, instead of reviewing shiny new toys by others
too early, spend leftover time to tie loose ends" attempts ;-)

I'd agree that the "final" version should do our usual "progressive
improvement, never stepping back one and then forward two", but I
wanted to see what the endgame state would look like first, and by
doing the incremental "the first one gets it 80% right, and fix it
up with follow-up patches" I didn't have to worry about at what
point I need to take the authorship of which patch.

Re: [PATCH v3 0/5] Expanding tabs in "git log" output

[Torsten Bögershausen]

>  [5/5] adds a new option --no-expand-tabs that controls the bit 4/5
>        introduces, so that "git log [--pretty] --no-expand-tabs"
>        would show the log message indented by 4 spaces, without tab
>        expansion.

Does this introduce an unnecessary regression out of the sudden ?

Would it make sense to have
git log --tab-size=8
(or similar)

and add a config variable like
git config ui.logtabsize
which is 0 by default to get the old handling and 8 for the new one ?

Re: [PATCH v3 0/5] Expanding tabs in "git log" output

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> writes:

>>  [5/5] adds a new option --no-expand-tabs that controls the bit 4/5
>>        introduces, so that "git log [--pretty] --no-expand-tabs"
>>        would show the log message indented by 4 spaces, without tab
>>        expansion.
>
> Does this introduce an unnecessary regression out of the sudden ?

As I see this as a pure UI-level improvement done to a few of our
Porcelain program, I would say there is no regression here.

Re: [PATCH v3 0/5] Expanding tabs in "git log" output

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> writes:

> Would it make sense to have
> git log --tab-size=8
> (or similar)
>
> and add a config variable like
> git config ui.logtabsize
> which is 0 by default to get the old handling and 8 for the new one ?

That may be a good approach (I agree with you that --tab-size is not
the best name).  Want to try it as a replacement?

Re: [PATCH v3 0/5] Expanding tabs in "git log" output

[Torsten Bögershausen]

On 2016-03-24 19.22, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> Would it make sense to have
>> git log --tab-size=8
>> (or similar)
>>
>> and add a config variable like
>> git config ui.logtabsize
>> which is 0 by default to get the old handling and 8 for the new one ?
> 
> That may be a good approach (I agree with you that --tab-size is not
> the best name).  Want to try it as a replacement?

May be log.tabwidth ?
I'm happy to help out here, if we can cook the feature in pu for 3-4 weeks
or so ?

Re: [PATCH v3 0/5] Expanding tabs in "git log" output

[Torsten Bögershausen]

This is copy-paste replacement for the last commit.
(Most probably it is white space damaged)
I'm not sure, is it's worth it ?
If yes, I can send a proper patch later.

git show HEAD

commit 3ac551127d51cd59b24f49729d9ce4dd011a09a1
Author: Junio C Hamano <gitster@pobox.com>
Date:   Wed Mar 23 15:57:42 2016 -0700

    pretty-print: Add the config variable log.tabwidth

    The output formats of "git log" that indent the log message by 4
    spaces have been updated to expand tabs by default in previous
    steps, without a way to restore the original behaviour.

    Introduce a config variable log.tabwidth to allow this.

        $ git -c log.tabwidth=0 log [--pretty=medium]

    would not expand.

    The non-expansion can be made permanent:
        $ git config log.tabwidth 0

    Or the TAB width can be changed like this:
        $ git config log.tabwidth 4

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2cd6bdd..611f5e4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1915,6 +1915,10 @@ log.showRoot::
 	Tools like linkgit:git-log[1] or linkgit:git-whatchanged[1], which
 	normally hide the root commit will now show it. True by default.

+log.tabWidth::
+	Sets the width of a TAB.  If 0, no TAB expansion is done.
+	8 by default.
+
 log.mailmap::
 	If true, makes linkgit:git-log[1], linkgit:git-show[1], and
 	linkgit:git-whatchanged[1] assume `--use-mailmap`.
diff --git a/cache.h b/cache.h
index b829410..fd115d2 100644
--- a/cache.h
+++ b/cache.h
@@ -649,6 +649,7 @@ extern int ignore_case;
 extern int assume_unchanged;
 extern int prefer_symlink_refs;
 extern int log_all_ref_updates;
+extern unsigned log_tab_width;
 extern int warn_ambiguous_refs;
 extern int warn_on_object_refname_ambiguity;
 extern int shared_repository;
diff --git a/config.c b/config.c
index 9ba40bc..e6aadfe 100644
--- a/config.c
+++ b/config.c
@@ -1030,6 +1030,11 @@ int git_default_config(const char *var, const char
*value, void *dummy)
 		pack_size_limit_cfg = git_config_ulong(var, value);
 		return 0;
 	}
+
+	if (!strcmp(var, "log.tabwidth")) {
+		log_tab_width = (unsigned)git_config_ulong(var, value);
+		return 0;
+	}
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
 }
diff --git a/environment.c b/environment.c
index 6dec9d0..3c72b44 100644
--- a/environment.c
+++ b/environment.c
@@ -21,6 +21,7 @@ int ignore_case;
 int assume_unchanged;
 int prefer_symlink_refs;
 int is_bare_repository_cfg = -1; /* unspecified */
+unsigned log_tab_width = 8;
 int log_all_ref_updates = -1; /* unspecified */
 int warn_ambiguous_refs = 1;
 int warn_on_object_refname_ambiguity = 1;
diff --git a/pretty.c b/pretty.c
index 5a33b7e..1d92c55 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1667,7 +1667,7 @@ static void strbuf_add_tabexpand(struct strbuf *sb,
 		strbuf_add(sb, line, tab - line);

 		/* .. and the de-tabified tab */
-		strbuf_addchars(sb, ' ', 8-(width & 7));
+		strbuf_addchars(sb, ' ', log_tab_width - (width % log_tab_width));

 		/* Skip over the printed part .. */
 		linelen -= 1+tab-line;
@@ -1692,7 +1692,7 @@ static void pp_handle_indent(struct pretty_print_context *pp,
 			     const char *line, int linelen)
 {
 	strbuf_addchars(sb, ' ', indent);
-	if (pp->expand_tabs_in_log)
+	if (pp->expand_tabs_in_log && log_tab_width)
 		strbuf_add_tabexpand(sb, line, linelen);
 	else
 		strbuf_add(sb, line, linelen);
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 96233ca..9235a2e 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -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 -c log.tabwidth=0 log HEAD >log &&
 	GIT_DIR=non-existing git shortlog -w <log >out &&
 	test_cmp expect out
 '

Re: [PATCH v3 0/5] Expanding tabs in "git log" output

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> writes:

> On 2016-03-24 19.22, Junio C Hamano wrote:
>> Torsten Bögershausen <tboegi@web.de> writes:
>> 
>>> Would it make sense to have
>>> git log --tab-size=8
>>> (or similar)
>>>
>>> and add a config variable like
>>> git config ui.logtabsize
>>> which is 0 by default to get the old handling and 8 for the new one ?
>> 
>> That may be a good approach (I agree with you that --tab-size is not
>> the best name).  Want to try it as a replacement?
>
> May be log.tabwidth ?

The reason why I thought "--tab-size=4" is not the best name is
because it only states that the user prefers a non-standard tab
stops that are every 4 display spaces, and does not say what we
with do that information, i.e. it does not hint "We expand tabs
to spaces according to this setting" in its name.

I think log.tabwidth shares the same characteristics.

Re: [PATCH v3 0/5] Expanding tabs in "git log" output

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> writes:

> This is copy-paste replacement for the last commit.
> (Most probably it is white space damaged)
> I'm not sure, is it's worth it ?

Not if you are keeping "expand_tabs_in_log" boolean field.

I was expecting that the new "log-tab-width" thing extends the
expand_tabs_in_log as the concept--it used to be a boolean "do we or
do we not expand?" to "set it to 0 if we do not want to expand, set
it to N if we do want to expand to every N display spaces".  In
other words, if you introduce this new thing, the boolean should not
e necessary and it should go.  Did I misread your earlier message
that described your idea?

> +log.tabWidth::
> +	Sets the width of a TAB.  If 0, no TAB expansion is done.
> +	8 by default.

You need to make it clear where tabs are expanded.  The readers
would wonder if it expands tabs in "log -p" patch output, etc.


A related tangent. I suspect

	git format-patch --expand-tabs-in-log-message=4

might be a good feature to help people whose editors are configured
to move to next-multiple-of-4 column with a tab, and applying their
patch would show unaligned lines in "git log" output for others, by
expanding their tabs when sending the patch out.  We might even want
to add a related option

	git am --unexpand-tabs

that collapses a run of SP that fills to next-multiple-of-8 into a
tab on the receiving end.

[PATCH v4 0/3] Expanding tabs in "git log" output

[Junio C Hamano]

So here is the fourth try.  Previous round are at $gmane/289694,
$gmane/289166, and $gmane/288987.

I didn't quite find a good order to build this progressively, so
this series:

 - First adds the internal machinery and explicit --expand-tabs.
   This keeps Linus's authorship, but is different in that it is not
   enabled by default;

 - Then enable --expand-tabs by default for selected pretty formats;

 - And optionally, allow custom tab-width to be used.

Junio C Hamano (2):
  pretty: enable --expand-tabs by default for selected pretty formats
  pretty: allow tweaking tabwidth in --expand-tabs

Linus Torvalds (1):
  pretty: expand tabs in indented logs to make things line up properly

 Documentation/pretty-options.txt | 14 +++++++
 commit.h                         |  1 +
 log-tree.c                       |  1 +
 pretty.c                         | 87 +++++++++++++++++++++++++++++++++++-----
 revision.c                       | 10 +++++
 revision.h                       |  2 +-
 t/t4201-shortlog.sh              |  2 +-
 7 files changed, 106 insertions(+), 11 deletions(-)

-- 
2.8.0-215-gd29a7d9

[PATCH v4 1/3] pretty: expand tabs in indented logs to make things line up properly

[Junio C Hamano]

From: Linus Torvalds <torvalds@linux-foundation.org>

A commit log message sometimes tries to line things up using tabs,
assuming fixed-width font with the standard 8-place tab settings.
Viewing such a commit however does not work well in "git log", as
we indent the lines by prefixing 4 spaces in front of them.

This should all line up:

  Column 1	Column 2
  --------	--------
  A		B
  ABCD		EFGH
  SPACES        Instead of Tabs

Even with multi-byte UTF8 characters:

  Column 1	Column 2
  --------	--------
  Ä		B
  åäö		100
  A Møøse	once bit my sister..

Tab-expand the lines in "git log --expand-tabs" output before
prefixing 4 spaces.

This is based on the patch by Linus Torvalds, but changed to require
an explicit command line option to enable the behaviour.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/pretty-options.txt |  6 ++++
 commit.h                         |  1 +
 log-tree.c                       |  1 +
 pretty.c                         | 71 ++++++++++++++++++++++++++++++++++++++--
 revision.c                       |  2 ++
 revision.h                       |  1 +
 6 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 4b659ac..4fb5c76 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -42,6 +42,12 @@ people using 80-column terminals.
 	verbatim; this means that invalid sequences in the original
 	commit may be copied to the output.
 
+--expand-tabs::
+	Perform a tab expansion (replace each tab with enough number
+	of spaces to fill to the next display column that is
+	multiple of 8) in the log message before using the message
+	to show in the output.
+
 ifndef::git-rev-list[]
 --notes[=<ref>]::
 	Show the notes (see linkgit:git-notes[1]) that annotate the
diff --git a/commit.h b/commit.h
index 5d58be0..a7ef682 100644
--- a/commit.h
+++ b/commit.h
@@ -147,6 +147,7 @@ struct pretty_print_context {
 	int preserve_subject;
 	struct date_mode date_mode;
 	unsigned date_mode_explicit:1;
+	unsigned expand_tabs_in_log:1;
 	int need_8bit_cte;
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
diff --git a/log-tree.c b/log-tree.c
index 60f9839..78a5381 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -683,6 +683,7 @@ void show_log(struct rev_info *opt)
 	ctx.fmt = opt->commit_format;
 	ctx.mailmap = opt->mailmap;
 	ctx.color = opt->diffopt.use_color;
+	ctx.expand_tabs_in_log = opt->expand_tabs_in_log;
 	ctx.output_encoding = get_log_output_encoding();
 	if (opt->from_ident.mail_begin && opt->from_ident.name_begin)
 		ctx.from_ident = &opt->from_ident;
diff --git a/pretty.c b/pretty.c
index 92b2870..c8b075d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1629,6 +1629,72 @@ void pp_title_line(struct pretty_print_context *pp,
 	strbuf_release(&title);
 }
 
+static int pp_utf8_width(const char *start, const char *end)
+{
+	int width = 0;
+	size_t remain = end - start;
+
+	while (remain) {
+		int n = utf8_width(&start, &remain);
+		if (n < 0 || !start)
+			return -1;
+		width += n;
+	}
+	return width;
+}
+
+static void strbuf_add_tabexpand(struct strbuf *sb,
+				 const char *line, int linelen)
+{
+	const char *tab;
+
+	while ((tab = memchr(line, '\t', linelen)) != NULL) {
+		int width = pp_utf8_width(line, tab);
+
+		/*
+		 * If it wasn't well-formed utf8, or it
+		 * had characters with badly defined
+		 * width (control characters etc), just
+		 * give up on trying to align things.
+		 */
+		if (width < 0)
+			break;
+
+		/* Output the data .. */
+		strbuf_add(sb, line, tab - line);
+
+		/* .. and the de-tabified tab */
+		strbuf_addchars(sb, ' ', 8 - (width % 8));
+
+		/* Skip over the printed part .. */
+		linelen -= tab + 1 - line;
+		line = tab + 1;
+	}
+
+	/*
+	 * Print out everything after the last tab without
+	 * worrying about width - there's nothing more to
+	 * align.
+	 */
+	strbuf_add(sb, line, linelen);
+}
+
+/*
+ * pp_handle_indent() prints out the intendation, and
+ * the whole line (without the final newline), after
+ * de-tabifying.
+ */
+static void pp_handle_indent(struct pretty_print_context *pp,
+			     struct strbuf *sb, int indent,
+			     const char *line, int linelen)
+{
+	strbuf_addchars(sb, ' ', indent);
+	if (pp->expand_tabs_in_log)
+		strbuf_add_tabexpand(sb, line, linelen);
+	else
+		strbuf_add(sb, line, linelen);
+}
+
 void pp_remainder(struct pretty_print_context *pp,
 		  const char **msg_p,
 		  struct strbuf *sb,
@@ -1653,8 +1719,9 @@ void pp_remainder(struct pretty_print_context *pp,
 
 		strbuf_grow(sb, linelen + indent + 20);
 		if (indent)
-			strbuf_addchars(sb, ' ', indent);
-		strbuf_add(sb, line, linelen);
+			pp_handle_indent(pp, sb, indent, line, linelen);
+		else
+			strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
 	}
 }
diff --git a/revision.c b/revision.c
index df56fce..e662230 100644
--- a/revision.c
+++ b/revision.c
@@ -1915,6 +1915,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->verbose_header = 1;
 		revs->pretty_given = 1;
 		get_commit_format(arg+9, revs);
+	} else if (!strcmp(arg, "--expand-tabs")) {
+		revs->expand_tabs_in_log = 1;
 	} else if (!strcmp(arg, "--show-notes") || !strcmp(arg, "--notes")) {
 		revs->show_notes = 1;
 		revs->show_notes_given = 1;
diff --git a/revision.h b/revision.h
index 23857c0..4079753 100644
--- a/revision.h
+++ b/revision.h
@@ -133,6 +133,7 @@ struct rev_info {
 			show_notes_given:1,
 			show_signature:1,
 			pretty_given:1,
+			expand_tabs_in_log:1,
 			abbrev_commit:1,
 			abbrev_commit_given:1,
 			zero_commit:1,
-- 
2.8.0-215-gd29a7d9

[PATCH v4 2/3] pretty: enable --expand-tabs by default for selected pretty formats

[Junio C Hamano]

"git log --pretty={medium,full,fuller}" and "git log" by default
prepend 4 spaces to the log message, so it makes sense to enable
the new "expand-tabs" facility by default for these formats.
Add --no-expand-tabs option to override the new default.

The change alone breaks a test in t4201 that runs "git shortlog"
on the output from "git log", and expects that the output from
"git log" does not do such a tab expansion.  Adjust the test to
explicitly disable expand-tabs with --no-expand-tabs.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/pretty-options.txt |  6 ++++++
 pretty.c                         | 16 +++++++++-------
 revision.c                       |  3 +++
 t/t4201-shortlog.sh              |  2 +-
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 4fb5c76..23967b6 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -43,10 +43,16 @@ people using 80-column terminals.
 	commit may be copied to the output.
 
 --expand-tabs::
+--no-expand-tabs::
 	Perform a tab expansion (replace each tab with enough number
 	of spaces to fill to the next display column that is
 	multiple of 8) in the log message before using the message
 	to show in the output.
++
+By default, tabs are expanded in pretty formats that indent the log
+message by 4 spaces (i.e.  'medium', which is the default, 'full',
+and "fuller').  `--no-expand-tabs` option can be used to disable
+this.
 
 ifndef::git-rev-list[]
 --notes[=<ref>]::
diff --git a/pretty.c b/pretty.c
index c8b075d..de22a8c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -16,6 +16,7 @@ static struct cmt_fmt_map {
 	const char *name;
 	enum cmit_fmt format;
 	int is_tformat;
+	int expand_tabs_in_log;
 	int is_alias;
 	const char *user_format;
 } *commit_formats;
@@ -87,13 +88,13 @@ static int git_pretty_formats_config(const char *var, const char *value, void *c
 static void setup_commit_formats(void)
 {
 	struct cmt_fmt_map builtin_formats[] = {
-		{ "raw",	CMIT_FMT_RAW,		0 },
-		{ "medium",	CMIT_FMT_MEDIUM,	0 },
-		{ "short",	CMIT_FMT_SHORT,		0 },
-		{ "email",	CMIT_FMT_EMAIL,		0 },
-		{ "fuller",	CMIT_FMT_FULLER,	0 },
-		{ "full",	CMIT_FMT_FULL,		0 },
-		{ "oneline",	CMIT_FMT_ONELINE,	1 }
+		{ "raw",	CMIT_FMT_RAW,		0,	0 },
+		{ "medium",	CMIT_FMT_MEDIUM,	0,	1 },
+		{ "short",	CMIT_FMT_SHORT,		0,	0 },
+		{ "email",	CMIT_FMT_EMAIL,		0,	0 },
+		{ "fuller",	CMIT_FMT_FULLER,	0,	1 },
+		{ "full",	CMIT_FMT_FULL,		0,	1 },
+		{ "oneline",	CMIT_FMT_ONELINE,	1,	0 }
 	};
 	commit_formats_len = ARRAY_SIZE(builtin_formats);
 	builtin_formats_len = commit_formats_len;
@@ -172,6 +173,7 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 
 	rev->commit_format = commit_format->format;
 	rev->use_terminator = commit_format->is_tformat;
+	rev->expand_tabs_in_log = commit_format->expand_tabs_in_log;
 	if (commit_format->format == CMIT_FMT_USERFORMAT) {
 		save_user_format(rev, commit_format->user_format,
 				 commit_format->is_tformat);
diff --git a/revision.c b/revision.c
index e662230..b1d767a 100644
--- a/revision.c
+++ b/revision.c
@@ -1412,6 +1412,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->skip_count = -1;
 	revs->max_count = -1;
 	revs->max_parents = -1;
+	revs->expand_tabs_in_log = 1;
 
 	revs->commit_format = CMIT_FMT_DEFAULT;
 
@@ -1917,6 +1918,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		get_commit_format(arg+9, revs);
 	} else if (!strcmp(arg, "--expand-tabs")) {
 		revs->expand_tabs_in_log = 1;
+	} else if (!strcmp(arg, "--no-expand-tabs")) {
+		revs->expand_tabs_in_log = 0;
 	} else if (!strcmp(arg, "--show-notes") || !strcmp(arg, "--notes")) {
 		revs->show_notes = 1;
 		revs->show_notes_given = 1;
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 7600a3e..2fec948 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -115,7 +115,7 @@ EOF
 '
 
 test_expect_success !MINGW 'shortlog from non-git directory' '
-	git log HEAD >log &&
+	git log --no-expand-tabs HEAD >log &&
 	GIT_DIR=non-existing git shortlog -w <log >out &&
 	test_cmp expect out
 '
-- 
2.8.0-215-gd29a7d9

[PATCH v4 3/3] pretty: allow tweaking tabwidth in --expand-tabs

[Junio C Hamano]

When the local convention of the project is to use tab width that is
not 8, it may make sense to allow "git log --expand-tabs=<n>" to
tweak the output to match it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/pretty-options.txt | 10 ++++++----
 commit.h                         |  2 +-
 pretty.c                         | 12 ++++++------
 revision.c                       |  9 +++++++--
 revision.h                       |  3 +--
 5 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 23967b6..8a944b1 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -42,17 +42,19 @@ people using 80-column terminals.
 	verbatim; this means that invalid sequences in the original
 	commit may be copied to the output.
 
+--expand-tabs=<n>::
 --expand-tabs::
 --no-expand-tabs::
 	Perform a tab expansion (replace each tab with enough number
 	of spaces to fill to the next display column that is
-	multiple of 8) in the log message before using the message
-	to show in the output.
+	multiple of '<n>') in the log message before using the message
+	to show in the output.  `--expand-tabs` is a short-hand for
+	`--expand-tabs=8`, and `--no-expand-tabs` is a short-hand for
+	`--expand-tabs=0`, which disables tab expansion.
 +
 By default, tabs are expanded in pretty formats that indent the log
 message by 4 spaces (i.e.  'medium', which is the default, 'full',
-and "fuller').  `--no-expand-tabs` option can be used to disable
-this.
+and "fuller').
 
 ifndef::git-rev-list[]
 --notes[=<ref>]::
diff --git a/commit.h b/commit.h
index a7ef682..2185c8d 100644
--- a/commit.h
+++ b/commit.h
@@ -147,7 +147,7 @@ struct pretty_print_context {
 	int preserve_subject;
 	struct date_mode date_mode;
 	unsigned date_mode_explicit:1;
-	unsigned expand_tabs_in_log:1;
+	unsigned expand_tabs_in_log;
 	int need_8bit_cte;
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
diff --git a/pretty.c b/pretty.c
index de22a8c..b340ecd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -89,11 +89,11 @@ static void setup_commit_formats(void)
 {
 	struct cmt_fmt_map builtin_formats[] = {
 		{ "raw",	CMIT_FMT_RAW,		0,	0 },
-		{ "medium",	CMIT_FMT_MEDIUM,	0,	1 },
+		{ "medium",	CMIT_FMT_MEDIUM,	0,	8 },
 		{ "short",	CMIT_FMT_SHORT,		0,	0 },
 		{ "email",	CMIT_FMT_EMAIL,		0,	0 },
-		{ "fuller",	CMIT_FMT_FULLER,	0,	1 },
-		{ "full",	CMIT_FMT_FULL,		0,	1 },
+		{ "fuller",	CMIT_FMT_FULLER,	0,	8 },
+		{ "full",	CMIT_FMT_FULL,		0,	8 },
 		{ "oneline",	CMIT_FMT_ONELINE,	1,	0 }
 	};
 	commit_formats_len = ARRAY_SIZE(builtin_formats);
@@ -1645,7 +1645,7 @@ static int pp_utf8_width(const char *start, const char *end)
 	return width;
 }
 
-static void strbuf_add_tabexpand(struct strbuf *sb,
+static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,
 				 const char *line, int linelen)
 {
 	const char *tab;
@@ -1666,7 +1666,7 @@ static void strbuf_add_tabexpand(struct strbuf *sb,
 		strbuf_add(sb, line, tab - line);
 
 		/* .. and the de-tabified tab */
-		strbuf_addchars(sb, ' ', 8 - (width % 8));
+		strbuf_addchars(sb, ' ', tabwidth - (width % tabwidth));
 
 		/* Skip over the printed part .. */
 		linelen -= tab + 1 - line;
@@ -1692,7 +1692,7 @@ static void pp_handle_indent(struct pretty_print_context *pp,
 {
 	strbuf_addchars(sb, ' ', indent);
 	if (pp->expand_tabs_in_log)
-		strbuf_add_tabexpand(sb, line, linelen);
+		strbuf_add_tabexpand(sb, pp->expand_tabs_in_log, line, linelen);
 	else
 		strbuf_add(sb, line, linelen);
 }
diff --git a/revision.c b/revision.c
index b1d767a..4f9ecbe 100644
--- a/revision.c
+++ b/revision.c
@@ -1412,7 +1412,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->skip_count = -1;
 	revs->max_count = -1;
 	revs->max_parents = -1;
-	revs->expand_tabs_in_log = 1;
+	revs->expand_tabs_in_log = 8;
 
 	revs->commit_format = CMIT_FMT_DEFAULT;
 
@@ -1917,9 +1917,14 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->pretty_given = 1;
 		get_commit_format(arg+9, revs);
 	} else if (!strcmp(arg, "--expand-tabs")) {
-		revs->expand_tabs_in_log = 1;
+		revs->expand_tabs_in_log = 8;
 	} else if (!strcmp(arg, "--no-expand-tabs")) {
 		revs->expand_tabs_in_log = 0;
+	} else if (skip_prefix(arg, "--expand-tabs=", &arg)) {
+		int val;
+		if (strtol_i(arg, 10, &val) < 0 || val < 0)
+			die("'%s': not a non-negative integer", arg);
+		revs->expand_tabs_in_log = val;
 	} else if (!strcmp(arg, "--show-notes") || !strcmp(arg, "--notes")) {
 		revs->show_notes = 1;
 		revs->show_notes_given = 1;
diff --git a/revision.h b/revision.h
index 4079753..cf6615a 100644
--- a/revision.h
+++ b/revision.h
@@ -133,7 +133,6 @@ struct rev_info {
 			show_notes_given:1,
 			show_signature:1,
 			pretty_given:1,
-			expand_tabs_in_log:1,
 			abbrev_commit:1,
 			abbrev_commit_given:1,
 			zero_commit:1,
@@ -149,7 +148,7 @@ struct rev_info {
 			linear:1;
 
 	struct date_mode date_mode;
-
+	unsigned int	expand_tabs_in_log;
 	unsigned int	abbrev;
 	enum cmit_fmt	commit_format;
 	struct log_info *loginfo;
-- 
2.8.0-215-gd29a7d9

Re: [PATCH v4 1/3] pretty: expand tabs in indented logs to make things line up properly

[Eric Sunshine]

On Tue, Mar 29, 2016 at 7:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> From: Linus Torvalds <torvalds@linux-foundation.org>
>
> A commit log message sometimes tries to line things up using tabs,
> assuming fixed-width font with the standard 8-place tab settings.
> Viewing such a commit however does not work well in "git log", as
> we indent the lines by prefixing 4 spaces in front of them.
> [...]
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
> @@ -42,6 +42,12 @@ people using 80-column terminals.
> +--expand-tabs::
> +       Perform a tab expansion (replace each tab with enough number
> +       of spaces to fill to the next display column that is

Nit: "enough spaces" or "a sufficient number of spaces".

> +       multiple of 8) in the log message before using the message
> +       to show in the output.

Re: [PATCH v4 2/3] pretty: enable --expand-tabs by default for selected pretty formats

[Jeff King]

On Tue, Mar 29, 2016 at 04:15:08PM -0700, Junio C Hamano wrote:

> diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
> index 4fb5c76..23967b6 100644
> --- a/Documentation/pretty-options.txt
> +++ b/Documentation/pretty-options.txt
> @@ -43,10 +43,16 @@ people using 80-column terminals.
>  	commit may be copied to the output.
>  
>  --expand-tabs::
> +--no-expand-tabs::
>  	Perform a tab expansion (replace each tab with enough number
>  	of spaces to fill to the next display column that is
>  	multiple of 8) in the log message before using the message
>  	to show in the output.
> ++
> +By default, tabs are expanded in pretty formats that indent the log
> +message by 4 spaces (i.e.  'medium', which is the default, 'full',
> +and "fuller').  `--no-expand-tabs` option can be used to disable
> +this.

Mismatched quote types on "fuller".

> @@ -172,6 +173,7 @@ void get_commit_format(const char *arg, struct rev_info *rev)
>  
>  	rev->commit_format = commit_format->format;
>  	rev->use_terminator = commit_format->is_tformat;
> +	rev->expand_tabs_in_log = commit_format->expand_tabs_in_log;
>  	if (commit_format->format == CMIT_FMT_USERFORMAT) {
>  		save_user_format(rev, commit_format->user_format,
>  				 commit_format->is_tformat);

This feels like the wrong time to set the value in rev_info, as it means
that:

  git log --no-expand-tabs --pretty=full

and

  git log --pretty=full --no-expand-tabs

behave differently. The other values set in get_commit_format, like
"use_terminator", are inherently part of the format, but I don't think
this is. We just want to set the default if the user did not express
another preference.

Likewise, if we were to eventually add config like "[log]expandtab = 4",
it should not be overridden by "--pretty=full" (but we probably _would_
want to have it kick in only for certain formats).

So I think we really want to set an alternate variable here, and then do
something like:

  if (rev->expand_tabs_in_log < 0)
	rev->expand_tabs_in_log = rev->commit_format_expand_tab_default;

-Peff

Re: [PATCH v4 1/3] pretty: expand tabs in indented logs to make things line up properly

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Tue, Mar 29, 2016 at 7:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> From: Linus Torvalds <torvalds@linux-foundation.org>
>>
>> A commit log message sometimes tries to line things up using tabs,
>> assuming fixed-width font with the standard 8-place tab settings.
>> Viewing such a commit however does not work well in "git log", as
>> we indent the lines by prefixing 4 spaces in front of them.
>> [...]
>> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
>> @@ -42,6 +42,12 @@ people using 80-column terminals.
>> +--expand-tabs::
>> +       Perform a tab expansion (replace each tab with enough number
>> +       of spaces to fill to the next display column that is
>
> Nit: "enough spaces" or "a sufficient number of spaces".

Thanks, will be part of a reroll (when it happens ;-).

Re: [PATCH v4 2/3] pretty: enable --expand-tabs by default for selected pretty formats

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Mar 29, 2016 at 04:15:08PM -0700, Junio C Hamano wrote:
>
>> diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
>> index 4fb5c76..23967b6 100644
>> --- a/Documentation/pretty-options.txt
>> +++ b/Documentation/pretty-options.txt
>> @@ -43,10 +43,16 @@ people using 80-column terminals.
>>  	commit may be copied to the output.
>>  
>>  --expand-tabs::
>> +--no-expand-tabs::
>>  	Perform a tab expansion (replace each tab with enough number
>>  	of spaces to fill to the next display column that is
>>  	multiple of 8) in the log message before using the message
>>  	to show in the output.
>> ++
>> +By default, tabs are expanded in pretty formats that indent the log
>> +message by 4 spaces (i.e.  'medium', which is the default, 'full',
>> +and "fuller').  `--no-expand-tabs` option can be used to disable
>> +this.
>
> Mismatched quote types on "fuller".

Thanks.

>> @@ -172,6 +173,7 @@ void get_commit_format(const char *arg, struct rev_info *rev)
>>  
>>  	rev->commit_format = commit_format->format;
>>  	rev->use_terminator = commit_format->is_tformat;
>> +	rev->expand_tabs_in_log = commit_format->expand_tabs_in_log;
>>  	if (commit_format->format == CMIT_FMT_USERFORMAT) {
>>  		save_user_format(rev, commit_format->user_format,
>>  				 commit_format->is_tformat);
>
> This feels like the wrong time to set the value in rev_info, as it means
> that:
>
>   git log --no-expand-tabs --pretty=full
>
> and
>
>   git log --pretty=full --no-expand-tabs
>
> behave differently.

I was sort of hoping that we can get away by defining that "an
explicit --pretty asks for the full behaviour of the format it
specifies, e.g. if you ask --pretty=medium, you are asking for
4-space indented tab-expanded log with the headers at the medium
level of detail".

> The other values set in get_commit_format, like "use_terminator",
> are inherently part of the format, but I don't think this is.

IOW, I was hoping nobody would agree with that and rather everybody
would consider tab-expansion is part of the format.

Let me try your way instead and report how it went when I send out a
reroll.

> Likewise, if we were to eventually add config like "[log]expandtab = 4",
> it should not be overridden by "--pretty=full" (but we probably _would_
> want to have it kick in only for certain formats).

This is exactly why I didn't do a configuration variable, as I think
we can make only 50% of people happy.  Some would say "when I
explicitly ask for the "email" format, I expect that expandtab
configuration gets ignored" while others would say "I said I want
expandtab in the configuration no matter what".

[PATCH v5 0/4] Expanding tabs in "git log" output

[Junio C Hamano]

So here is the fifth and hopefully the final try.  Previous round
are at $gmane/289694, $gmane/289166, $gmane/288987 and
$gmane/290222.

This round is different from v4 in the following ways.

 * wording changes, grammo- and typo-fixes in the documentation
   (thanks to Eric Sunshine and Jeff King).

 * v4 made --pretty=$fmt to override an earlier --expand-tabs=<n>;
   this round only allows --pretty=$fmt to set the default behaviour
   when an explicit --expand-tabs=<n> is given on the command line
   (thanks to Jeff King).
   
 * comes with a test.

See the end of this cover letter for an interdiff.

Junio C Hamano (3):
  pretty: enable --expand-tabs by default for selected pretty formats
  pretty: allow tweaking tabwidth in --expand-tabs
  pretty: test --expand-tabs

Linus Torvalds (1):
  pretty: expand tabs in indented logs to make things line up properly

 Documentation/pretty-options.txt | 14 ++++++
 builtin/log.c                    |  1 +
 commit.h                         |  1 +
 log-tree.c                       |  1 +
 pretty.c                         | 90 ++++++++++++++++++++++++++++++++----
 revision.c                       | 14 ++++++
 revision.h                       |  2 +
 t/t4201-shortlog.sh              |  2 +-
 t/t4213-log-tabexpand.sh         | 98 ++++++++++++++++++++++++++++++++++++++++
 9 files changed, 213 insertions(+), 10 deletions(-)
 create mode 100755 t/t4213-log-tabexpand.sh

-- 
2.8.1-251-g9997610


diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 8a944b1..93ad1cd 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -45,16 +45,16 @@ people using 80-column terminals.
 --expand-tabs=<n>::
 --expand-tabs::
 --no-expand-tabs::
-	Perform a tab expansion (replace each tab with enough number
-	of spaces to fill to the next display column that is
-	multiple of '<n>') in the log message before using the message
-	to show in the output.  `--expand-tabs` is a short-hand for
-	`--expand-tabs=8`, and `--no-expand-tabs` is a short-hand for
-	`--expand-tabs=0`, which disables tab expansion.
+	Perform a tab expansion (replace each tab with enough spaces
+	to fill to the next display column that is multiple of '<n>')
+	in the log message before showing it in the output.
+	`--expand-tabs` is a short-hand for `--expand-tabs=8`, and
+	`--no-expand-tabs` is a short-hand for `--expand-tabs=0`,
+	which disables tab expansion.
 +
 By default, tabs are expanded in pretty formats that indent the log
 message by 4 spaces (i.e.  'medium', which is the default, 'full',
-and "fuller').
+and 'fuller').
 
 ifndef::git-rev-list[]
 --notes[=<ref>]::
diff --git a/builtin/log.c b/builtin/log.c
index e00cea7..e5775ae 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1281,6 +1281,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	git_config(git_format_config, NULL);
 	init_revisions(&rev, prefix);
 	rev.commit_format = CMIT_FMT_EMAIL;
+	rev.expand_tabs_in_log_default = 0;
 	rev.verbose_header = 1;
 	rev.diff = 1;
 	rev.max_parents = 1;
diff --git a/commit.h b/commit.h
index 2185c8d..b06db4d 100644
--- a/commit.h
+++ b/commit.h
@@ -147,7 +147,7 @@ struct pretty_print_context {
 	int preserve_subject;
 	struct date_mode date_mode;
 	unsigned date_mode_explicit:1;
-	unsigned expand_tabs_in_log;
+	int expand_tabs_in_log;
 	int need_8bit_cte;
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
diff --git a/pretty.c b/pretty.c
index b340ecd..87c4497 100644
--- a/pretty.c
+++ b/pretty.c
@@ -173,7 +173,7 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 
 	rev->commit_format = commit_format->format;
 	rev->use_terminator = commit_format->is_tformat;
-	rev->expand_tabs_in_log = commit_format->expand_tabs_in_log;
+	rev->expand_tabs_in_log_default = commit_format->expand_tabs_in_log;
 	if (commit_format->format == CMIT_FMT_USERFORMAT) {
 		save_user_format(rev, commit_format->user_format,
 				 commit_format->is_tformat);
@@ -1722,6 +1722,9 @@ void pp_remainder(struct pretty_print_context *pp,
 		strbuf_grow(sb, linelen + indent + 20);
 		if (indent)
 			pp_handle_indent(pp, sb, indent, line, linelen);
+		else if (pp->expand_tabs_in_log)
+			strbuf_add_tabexpand(sb, pp->expand_tabs_in_log,
+					     line, linelen);
 		else
 			strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
diff --git a/revision.c b/revision.c
index 4f9ecbe..47e9ee7 100644
--- a/revision.c
+++ b/revision.c
@@ -1412,9 +1412,10 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->skip_count = -1;
 	revs->max_count = -1;
 	revs->max_parents = -1;
-	revs->expand_tabs_in_log = 8;
+	revs->expand_tabs_in_log = -1;
 
 	revs->commit_format = CMIT_FMT_DEFAULT;
+	revs->expand_tabs_in_log_default = 8;
 
 	init_grep_defaults();
 	grep_init(&revs->grep_filter, prefix);
@@ -2398,6 +2399,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->first_parent_only && revs->bisect)
 		die(_("--first-parent is incompatible with --bisect"));
 
+	if (revs->expand_tabs_in_log < 0)
+		revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
+
 	return left;
 }
 
diff --git a/revision.h b/revision.h
index cf6615a..6cc36b4 100644
--- a/revision.h
+++ b/revision.h
@@ -148,7 +148,9 @@ struct rev_info {
 			linear:1;
 
 	struct date_mode date_mode;
-	unsigned int	expand_tabs_in_log;
+	int		expand_tabs_in_log; /* unset if negative */
+	int		expand_tabs_in_log_default;
+
 	unsigned int	abbrev;
 	enum cmit_fmt	commit_format;
 	struct log_info *loginfo;
diff --git a/t/t4213-log-tabexpand.sh b/t/t4213-log-tabexpand.sh
new file mode 100755
index 0000000..74ca03a
--- /dev/null
+++ b/t/t4213-log-tabexpand.sh
@@ -0,0 +1,98 @@
+#!/bin/sh
+
+test_description='log/show --expand-tabs'
+
+. ./test-lib.sh
+
+HT="	"
+title='tab indent at the beginning of the title line'
+body='tab indent on a line in the body'
+
+count_expand ()
+{
+	case " $* " in
+	*' --pretty=short '*)
+		line=$title ;;
+	*)
+		line=$body ;;
+	esac
+	expect=
+	count=$(( $1 + $2 )) ;# expected spaces
+	while test $count -gt 0
+	do
+		expect="$expect "
+		count=$(( $count - 1 ))
+	done
+	shift 2
+	count=$1 ;# expected tabs
+	while test $count -gt 0
+	do
+		expect="$expect$HT"
+		count=$(( $count - 1 ))
+	done
+	shift
+	{
+		echo "git show -s $*"
+		echo "$expect$line"
+	} | sed -e 's/ /./g' >expect
+
+	{
+		echo "git show -s $*"
+		git show -s "$@" |
+		sed -n -e "/$line\$/p"
+	} | sed -e 's/ /./g' >actual
+
+	test_cmp expect actual
+}
+
+test_expand ()
+{
+	fmt=$1
+	case "$fmt" in
+	*=raw | *=short | *=email)
+		default="0 1" ;;
+	*)
+		default="8 0" ;;
+	esac
+	case "$fmt" in
+	*=email)
+		in=0 ;;
+	*)
+		in=4 ;;
+	esac
+	test_expect_success "expand/no-expand${fmt:+ for $fmt}" '
+		count_expand $in $default $fmt &&
+		count_expand $in 8 0 $fmt --expand-tabs &&
+		count_expand $in 8 0 --expand-tabs $fmt &&
+		count_expand $in 8 0 $fmt --expand-tabs=8 &&
+		count_expand $in 8 0 --expand-tabs=8 $fmt &&
+		count_expand $in 0 1 $fmt --no-expand-tabs &&
+		count_expand $in 0 1 --no-expand-tabs $fmt &&
+		count_expand $in 0 1 $fmt --expand-tabs=0 &&
+		count_expand $in 0 1 --expand-tabs=0 $fmt &&
+		count_expand $in 4 0 $fmt --expand-tabs=4 &&
+		count_expand $in 4 0 --expand-tabs=4 $fmt
+	'
+}
+
+test_expect_success 'setup' '
+	test_tick &&
+	sed -e "s/Q/$HT/g" <<-EOF >msg &&
+	Q$title
+
+	Q$body
+	EOF
+	git commit --allow-empty -F msg
+'
+
+test_expand ""
+test_expand --pretty
+test_expand --pretty=short
+test_expand --pretty=medium
+test_expand --pretty=full
+test_expand --pretty=fuller
+test_expand --pretty=fuller
+test_expand --pretty=raw
+test_expand --pretty=email
+
+test_done

[PATCH v5 1/4] pretty: expand tabs in indented logs to make things line up properly

[Junio C Hamano]

From: Linus Torvalds <torvalds@linux-foundation.org>

A commit log message sometimes tries to line things up using tabs,
assuming fixed-width font with the standard 8-place tab settings.
Viewing such a commit however does not work well in "git log", as
we indent the lines by prefixing 4 spaces in front of them.

This should all line up:

  Column 1	Column 2
  --------	--------
  A		B
  ABCD		EFGH
  SPACES        Instead of Tabs

Even with multi-byte UTF8 characters:

  Column 1	Column 2
  --------	--------
  Ä		B
  åäö		100
  A Møøse	once bit my sister..

Tab-expand the lines in "git log --expand-tabs" output before
prefixing 4 spaces.

This is based on the patch by Linus Torvalds, but at this step, we
require an explicit command line option to enable the behaviour.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/pretty-options.txt |  5 +++
 commit.h                         |  1 +
 log-tree.c                       |  1 +
 pretty.c                         | 71 ++++++++++++++++++++++++++++++++++++++--
 revision.c                       |  2 ++
 revision.h                       |  1 +
 6 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 4b659ac..d820653 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -42,6 +42,11 @@ people using 80-column terminals.
 	verbatim; this means that invalid sequences in the original
 	commit may be copied to the output.
 
+--expand-tabs::
+	Perform a tab expansion (replace each tab with enough spaces
+	to fill to the next display column that is multiple of 8)
+	in the log message before showing it in the output.
+
 ifndef::git-rev-list[]
 --notes[=<ref>]::
 	Show the notes (see linkgit:git-notes[1]) that annotate the
diff --git a/commit.h b/commit.h
index 5d58be0..a7ef682 100644
--- a/commit.h
+++ b/commit.h
@@ -147,6 +147,7 @@ struct pretty_print_context {
 	int preserve_subject;
 	struct date_mode date_mode;
 	unsigned date_mode_explicit:1;
+	unsigned expand_tabs_in_log:1;
 	int need_8bit_cte;
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
diff --git a/log-tree.c b/log-tree.c
index 60f9839..78a5381 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -683,6 +683,7 @@ void show_log(struct rev_info *opt)
 	ctx.fmt = opt->commit_format;
 	ctx.mailmap = opt->mailmap;
 	ctx.color = opt->diffopt.use_color;
+	ctx.expand_tabs_in_log = opt->expand_tabs_in_log;
 	ctx.output_encoding = get_log_output_encoding();
 	if (opt->from_ident.mail_begin && opt->from_ident.name_begin)
 		ctx.from_ident = &opt->from_ident;
diff --git a/pretty.c b/pretty.c
index 92b2870..c8b075d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1629,6 +1629,72 @@ void pp_title_line(struct pretty_print_context *pp,
 	strbuf_release(&title);
 }
 
+static int pp_utf8_width(const char *start, const char *end)
+{
+	int width = 0;
+	size_t remain = end - start;
+
+	while (remain) {
+		int n = utf8_width(&start, &remain);
+		if (n < 0 || !start)
+			return -1;
+		width += n;
+	}
+	return width;
+}
+
+static void strbuf_add_tabexpand(struct strbuf *sb,
+				 const char *line, int linelen)
+{
+	const char *tab;
+
+	while ((tab = memchr(line, '\t', linelen)) != NULL) {
+		int width = pp_utf8_width(line, tab);
+
+		/*
+		 * If it wasn't well-formed utf8, or it
+		 * had characters with badly defined
+		 * width (control characters etc), just
+		 * give up on trying to align things.
+		 */
+		if (width < 0)
+			break;
+
+		/* Output the data .. */
+		strbuf_add(sb, line, tab - line);
+
+		/* .. and the de-tabified tab */
+		strbuf_addchars(sb, ' ', 8 - (width % 8));
+
+		/* Skip over the printed part .. */
+		linelen -= tab + 1 - line;
+		line = tab + 1;
+	}
+
+	/*
+	 * Print out everything after the last tab without
+	 * worrying about width - there's nothing more to
+	 * align.
+	 */
+	strbuf_add(sb, line, linelen);
+}
+
+/*
+ * pp_handle_indent() prints out the intendation, and
+ * the whole line (without the final newline), after
+ * de-tabifying.
+ */
+static void pp_handle_indent(struct pretty_print_context *pp,
+			     struct strbuf *sb, int indent,
+			     const char *line, int linelen)
+{
+	strbuf_addchars(sb, ' ', indent);
+	if (pp->expand_tabs_in_log)
+		strbuf_add_tabexpand(sb, line, linelen);
+	else
+		strbuf_add(sb, line, linelen);
+}
+
 void pp_remainder(struct pretty_print_context *pp,
 		  const char **msg_p,
 		  struct strbuf *sb,
@@ -1653,8 +1719,9 @@ void pp_remainder(struct pretty_print_context *pp,
 
 		strbuf_grow(sb, linelen + indent + 20);
 		if (indent)
-			strbuf_addchars(sb, ' ', indent);
-		strbuf_add(sb, line, linelen);
+			pp_handle_indent(pp, sb, indent, line, linelen);
+		else
+			strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
 	}
 }
diff --git a/revision.c b/revision.c
index df56fce..e662230 100644
--- a/revision.c
+++ b/revision.c
@@ -1915,6 +1915,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->verbose_header = 1;
 		revs->pretty_given = 1;
 		get_commit_format(arg+9, revs);
+	} else if (!strcmp(arg, "--expand-tabs")) {
+		revs->expand_tabs_in_log = 1;
 	} else if (!strcmp(arg, "--show-notes") || !strcmp(arg, "--notes")) {
 		revs->show_notes = 1;
 		revs->show_notes_given = 1;
diff --git a/revision.h b/revision.h
index 23857c0..4079753 100644
--- a/revision.h
+++ b/revision.h
@@ -133,6 +133,7 @@ struct rev_info {
 			show_notes_given:1,
 			show_signature:1,
 			pretty_given:1,
+			expand_tabs_in_log:1,
 			abbrev_commit:1,
 			abbrev_commit_given:1,
 			zero_commit:1,
-- 
2.8.1-251-g9997610

[PATCH v5 2/4] pretty: enable --expand-tabs by default for selected pretty formats

[Junio C Hamano]

"git log --pretty={medium,full,fuller}" and "git log" by default
prepend 4 spaces to the log message, so it makes sense to enable
the new "expand-tabs" facility by default for these formats.
Add --no-expand-tabs option to override the new default.

The change alone breaks a test in t4201 that runs "git shortlog"
on the output from "git log", and expects that the output from
"git log" does not do such a tab expansion.  Adjust the test to
explicitly disable expand-tabs with --no-expand-tabs.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/pretty-options.txt |  6 ++++++
 builtin/log.c                    |  1 +
 pretty.c                         | 18 +++++++++++-------
 revision.c                       |  7 +++++++
 revision.h                       |  3 ++-
 t/t4201-shortlog.sh              |  2 +-
 6 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index d820653..edbb02f 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -43,9 +43,15 @@ people using 80-column terminals.
 	commit may be copied to the output.
 
 --expand-tabs::
+--no-expand-tabs::
 	Perform a tab expansion (replace each tab with enough spaces
 	to fill to the next display column that is multiple of 8)
 	in the log message before showing it in the output.
++
+By default, tabs are expanded in pretty formats that indent the log
+message by 4 spaces (i.e.  'medium', which is the default, 'full',
+and 'fuller').  `--no-expand-tabs` option can be used to disable
+this.
 
 ifndef::git-rev-list[]
 --notes[=<ref>]::
diff --git a/builtin/log.c b/builtin/log.c
index e00cea7..e5775ae 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1281,6 +1281,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	git_config(git_format_config, NULL);
 	init_revisions(&rev, prefix);
 	rev.commit_format = CMIT_FMT_EMAIL;
+	rev.expand_tabs_in_log_default = 0;
 	rev.verbose_header = 1;
 	rev.diff = 1;
 	rev.max_parents = 1;
diff --git a/pretty.c b/pretty.c
index c8b075d..b7938e0 100644
--- a/pretty.c
+++ b/pretty.c
@@ -16,6 +16,7 @@ static struct cmt_fmt_map {
 	const char *name;
 	enum cmit_fmt format;
 	int is_tformat;
+	int expand_tabs_in_log;
 	int is_alias;
 	const char *user_format;
 } *commit_formats;
@@ -87,13 +88,13 @@ static int git_pretty_formats_config(const char *var, const char *value, void *c
 static void setup_commit_formats(void)
 {
 	struct cmt_fmt_map builtin_formats[] = {
-		{ "raw",	CMIT_FMT_RAW,		0 },
-		{ "medium",	CMIT_FMT_MEDIUM,	0 },
-		{ "short",	CMIT_FMT_SHORT,		0 },
-		{ "email",	CMIT_FMT_EMAIL,		0 },
-		{ "fuller",	CMIT_FMT_FULLER,	0 },
-		{ "full",	CMIT_FMT_FULL,		0 },
-		{ "oneline",	CMIT_FMT_ONELINE,	1 }
+		{ "raw",	CMIT_FMT_RAW,		0,	0 },
+		{ "medium",	CMIT_FMT_MEDIUM,	0,	1 },
+		{ "short",	CMIT_FMT_SHORT,		0,	0 },
+		{ "email",	CMIT_FMT_EMAIL,		0,	0 },
+		{ "fuller",	CMIT_FMT_FULLER,	0,	1 },
+		{ "full",	CMIT_FMT_FULL,		0,	1 },
+		{ "oneline",	CMIT_FMT_ONELINE,	1,	0 }
 	};
 	commit_formats_len = ARRAY_SIZE(builtin_formats);
 	builtin_formats_len = commit_formats_len;
@@ -172,6 +173,7 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 
 	rev->commit_format = commit_format->format;
 	rev->use_terminator = commit_format->is_tformat;
+	rev->expand_tabs_in_log_default = commit_format->expand_tabs_in_log;
 	if (commit_format->format == CMIT_FMT_USERFORMAT) {
 		save_user_format(rev, commit_format->user_format,
 				 commit_format->is_tformat);
@@ -1720,6 +1722,8 @@ void pp_remainder(struct pretty_print_context *pp,
 		strbuf_grow(sb, linelen + indent + 20);
 		if (indent)
 			pp_handle_indent(pp, sb, indent, line, linelen);
+		else if (pp->expand_tabs_in_log)
+			strbuf_add_tabexpand(sb, line, linelen);
 		else
 			strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
diff --git a/revision.c b/revision.c
index e662230..da53b6c 100644
--- a/revision.c
+++ b/revision.c
@@ -1412,8 +1412,10 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->skip_count = -1;
 	revs->max_count = -1;
 	revs->max_parents = -1;
+	revs->expand_tabs_in_log = -1;
 
 	revs->commit_format = CMIT_FMT_DEFAULT;
+	revs->expand_tabs_in_log_default = 1;
 
 	init_grep_defaults();
 	grep_init(&revs->grep_filter, prefix);
@@ -1917,6 +1919,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		get_commit_format(arg+9, revs);
 	} else if (!strcmp(arg, "--expand-tabs")) {
 		revs->expand_tabs_in_log = 1;
+	} else if (!strcmp(arg, "--no-expand-tabs")) {
+		revs->expand_tabs_in_log = 0;
 	} else if (!strcmp(arg, "--show-notes") || !strcmp(arg, "--notes")) {
 		revs->show_notes = 1;
 		revs->show_notes_given = 1;
@@ -2390,6 +2394,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->first_parent_only && revs->bisect)
 		die(_("--first-parent is incompatible with --bisect"));
 
+	if (revs->expand_tabs_in_log < 0)
+		revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
+
 	return left;
 }
 
diff --git a/revision.h b/revision.h
index 4079753..6cc36b4 100644
--- a/revision.h
+++ b/revision.h
@@ -133,7 +133,6 @@ struct rev_info {
 			show_notes_given:1,
 			show_signature:1,
 			pretty_given:1,
-			expand_tabs_in_log:1,
 			abbrev_commit:1,
 			abbrev_commit_given:1,
 			zero_commit:1,
@@ -149,6 +148,8 @@ struct rev_info {
 			linear:1;
 
 	struct date_mode date_mode;
+	int		expand_tabs_in_log; /* unset if negative */
+	int		expand_tabs_in_log_default;
 
 	unsigned int	abbrev;
 	enum cmit_fmt	commit_format;
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 7600a3e..2fec948 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -115,7 +115,7 @@ EOF
 '
 
 test_expect_success !MINGW 'shortlog from non-git directory' '
-	git log HEAD >log &&
+	git log --no-expand-tabs HEAD >log &&
 	GIT_DIR=non-existing git shortlog -w <log >out &&
 	test_cmp expect out
 '
-- 
2.8.1-251-g9997610

[PATCH v5 3/4] pretty: allow tweaking tabwidth in --expand-tabs

[Junio C Hamano]

When the local convention of the project is to use tab width that is
not 8, it may make sense to allow "git log --expand-tabs=<n>" to
tweak the output to match it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/pretty-options.txt |  9 ++++++---
 commit.h                         |  2 +-
 pretty.c                         | 15 ++++++++-------
 revision.c                       |  9 +++++++--
 4 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index edbb02f..93ad1cd 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -42,16 +42,19 @@ people using 80-column terminals.
 	verbatim; this means that invalid sequences in the original
 	commit may be copied to the output.
 
+--expand-tabs=<n>::
 --expand-tabs::
 --no-expand-tabs::
 	Perform a tab expansion (replace each tab with enough spaces
-	to fill to the next display column that is multiple of 8)
+	to fill to the next display column that is multiple of '<n>')
 	in the log message before showing it in the output.
+	`--expand-tabs` is a short-hand for `--expand-tabs=8`, and
+	`--no-expand-tabs` is a short-hand for `--expand-tabs=0`,
+	which disables tab expansion.
 +
 By default, tabs are expanded in pretty formats that indent the log
 message by 4 spaces (i.e.  'medium', which is the default, 'full',
-and 'fuller').  `--no-expand-tabs` option can be used to disable
-this.
+and 'fuller').
 
 ifndef::git-rev-list[]
 --notes[=<ref>]::
diff --git a/commit.h b/commit.h
index a7ef682..b06db4d 100644
--- a/commit.h
+++ b/commit.h
@@ -147,7 +147,7 @@ struct pretty_print_context {
 	int preserve_subject;
 	struct date_mode date_mode;
 	unsigned date_mode_explicit:1;
-	unsigned expand_tabs_in_log:1;
+	int expand_tabs_in_log;
 	int need_8bit_cte;
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
diff --git a/pretty.c b/pretty.c
index b7938e0..87c4497 100644
--- a/pretty.c
+++ b/pretty.c
@@ -89,11 +89,11 @@ static void setup_commit_formats(void)
 {
 	struct cmt_fmt_map builtin_formats[] = {
 		{ "raw",	CMIT_FMT_RAW,		0,	0 },
-		{ "medium",	CMIT_FMT_MEDIUM,	0,	1 },
+		{ "medium",	CMIT_FMT_MEDIUM,	0,	8 },
 		{ "short",	CMIT_FMT_SHORT,		0,	0 },
 		{ "email",	CMIT_FMT_EMAIL,		0,	0 },
-		{ "fuller",	CMIT_FMT_FULLER,	0,	1 },
-		{ "full",	CMIT_FMT_FULL,		0,	1 },
+		{ "fuller",	CMIT_FMT_FULLER,	0,	8 },
+		{ "full",	CMIT_FMT_FULL,		0,	8 },
 		{ "oneline",	CMIT_FMT_ONELINE,	1,	0 }
 	};
 	commit_formats_len = ARRAY_SIZE(builtin_formats);
@@ -1645,7 +1645,7 @@ static int pp_utf8_width(const char *start, const char *end)
 	return width;
 }
 
-static void strbuf_add_tabexpand(struct strbuf *sb,
+static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,
 				 const char *line, int linelen)
 {
 	const char *tab;
@@ -1666,7 +1666,7 @@ static void strbuf_add_tabexpand(struct strbuf *sb,
 		strbuf_add(sb, line, tab - line);
 
 		/* .. and the de-tabified tab */
-		strbuf_addchars(sb, ' ', 8 - (width % 8));
+		strbuf_addchars(sb, ' ', tabwidth - (width % tabwidth));
 
 		/* Skip over the printed part .. */
 		linelen -= tab + 1 - line;
@@ -1692,7 +1692,7 @@ static void pp_handle_indent(struct pretty_print_context *pp,
 {
 	strbuf_addchars(sb, ' ', indent);
 	if (pp->expand_tabs_in_log)
-		strbuf_add_tabexpand(sb, line, linelen);
+		strbuf_add_tabexpand(sb, pp->expand_tabs_in_log, line, linelen);
 	else
 		strbuf_add(sb, line, linelen);
 }
@@ -1723,7 +1723,8 @@ void pp_remainder(struct pretty_print_context *pp,
 		if (indent)
 			pp_handle_indent(pp, sb, indent, line, linelen);
 		else if (pp->expand_tabs_in_log)
-			strbuf_add_tabexpand(sb, line, linelen);
+			strbuf_add_tabexpand(sb, pp->expand_tabs_in_log,
+					     line, linelen);
 		else
 			strbuf_add(sb, line, linelen);
 		strbuf_addch(sb, '\n');
diff --git a/revision.c b/revision.c
index da53b6c..47e9ee7 100644
--- a/revision.c
+++ b/revision.c
@@ -1415,7 +1415,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->expand_tabs_in_log = -1;
 
 	revs->commit_format = CMIT_FMT_DEFAULT;
-	revs->expand_tabs_in_log_default = 1;
+	revs->expand_tabs_in_log_default = 8;
 
 	init_grep_defaults();
 	grep_init(&revs->grep_filter, prefix);
@@ -1918,9 +1918,14 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->pretty_given = 1;
 		get_commit_format(arg+9, revs);
 	} else if (!strcmp(arg, "--expand-tabs")) {
-		revs->expand_tabs_in_log = 1;
+		revs->expand_tabs_in_log = 8;
 	} else if (!strcmp(arg, "--no-expand-tabs")) {
 		revs->expand_tabs_in_log = 0;
+	} else if (skip_prefix(arg, "--expand-tabs=", &arg)) {
+		int val;
+		if (strtol_i(arg, 10, &val) < 0 || val < 0)
+			die("'%s': not a non-negative integer", arg);
+		revs->expand_tabs_in_log = val;
 	} else if (!strcmp(arg, "--show-notes") || !strcmp(arg, "--notes")) {
 		revs->show_notes = 1;
 		revs->show_notes_given = 1;
-- 
2.8.1-251-g9997610

[PATCH v5 4/4] pretty: test --expand-tabs

[Junio C Hamano]

The test prepares a simple commit with HT on its log message lines,
and makes sure that

 - formats that should or should not expand tabs by default do or do
   not expand tabs respectively,

 - with explicit --expand-tabs=<N> and short-hands --expand-tabs
   (equivalent to --expand-tabs=8) and --no-expand-tabs (equivalent
   to --expand-tabs=0) before or after the explicit --pretty=$fmt,
   the tabs are expanded (or not expanded) accordingly.

The tests use the second line of the log message for formats other
than --pretty=short, primarily because the first line of the email
format is handled specially to add the [PATCH] prefix, etc. in a
separate codepath (--pretty=short uses the first line because there
is no other line to test).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4213-log-tabexpand.sh | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)
 create mode 100755 t/t4213-log-tabexpand.sh

diff --git a/t/t4213-log-tabexpand.sh b/t/t4213-log-tabexpand.sh
new file mode 100755
index 0000000..74ca03a
--- /dev/null
+++ b/t/t4213-log-tabexpand.sh
@@ -0,0 +1,98 @@
+#!/bin/sh
+
+test_description='log/show --expand-tabs'
+
+. ./test-lib.sh
+
+HT="	"
+title='tab indent at the beginning of the title line'
+body='tab indent on a line in the body'
+
+count_expand ()
+{
+	case " $* " in
+	*' --pretty=short '*)
+		line=$title ;;
+	*)
+		line=$body ;;
+	esac
+	expect=
+	count=$(( $1 + $2 )) ;# expected spaces
+	while test $count -gt 0
+	do
+		expect="$expect "
+		count=$(( $count - 1 ))
+	done
+	shift 2
+	count=$1 ;# expected tabs
+	while test $count -gt 0
+	do
+		expect="$expect$HT"
+		count=$(( $count - 1 ))
+	done
+	shift
+	{
+		echo "git show -s $*"
+		echo "$expect$line"
+	} | sed -e 's/ /./g' >expect
+
+	{
+		echo "git show -s $*"
+		git show -s "$@" |
+		sed -n -e "/$line\$/p"
+	} | sed -e 's/ /./g' >actual
+
+	test_cmp expect actual
+}
+
+test_expand ()
+{
+	fmt=$1
+	case "$fmt" in
+	*=raw | *=short | *=email)
+		default="0 1" ;;
+	*)
+		default="8 0" ;;
+	esac
+	case "$fmt" in
+	*=email)
+		in=0 ;;
+	*)
+		in=4 ;;
+	esac
+	test_expect_success "expand/no-expand${fmt:+ for $fmt}" '
+		count_expand $in $default $fmt &&
+		count_expand $in 8 0 $fmt --expand-tabs &&
+		count_expand $in 8 0 --expand-tabs $fmt &&
+		count_expand $in 8 0 $fmt --expand-tabs=8 &&
+		count_expand $in 8 0 --expand-tabs=8 $fmt &&
+		count_expand $in 0 1 $fmt --no-expand-tabs &&
+		count_expand $in 0 1 --no-expand-tabs $fmt &&
+		count_expand $in 0 1 $fmt --expand-tabs=0 &&
+		count_expand $in 0 1 --expand-tabs=0 $fmt &&
+		count_expand $in 4 0 $fmt --expand-tabs=4 &&
+		count_expand $in 4 0 --expand-tabs=4 $fmt
+	'
+}
+
+test_expect_success 'setup' '
+	test_tick &&
+	sed -e "s/Q/$HT/g" <<-EOF >msg &&
+	Q$title
+
+	Q$body
+	EOF
+	git commit --allow-empty -F msg
+'
+
+test_expand ""
+test_expand --pretty
+test_expand --pretty=short
+test_expand --pretty=medium
+test_expand --pretty=full
+test_expand --pretty=fuller
+test_expand --pretty=fuller
+test_expand --pretty=raw
+test_expand --pretty=email
+
+test_done
-- 
2.8.1-251-g9997610

Re: [PATCH v5 4/4] pretty: test --expand-tabs

[Eric Sunshine]

On Mon, Apr 4, 2016 at 8:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The test prepares a simple commit with HT on its log message lines,
> and makes sure that
>
>  - formats that should or should not expand tabs by default do or do
>    not expand tabs respectively,
>
>  - with explicit --expand-tabs=<N> and short-hands --expand-tabs
>    (equivalent to --expand-tabs=8) and --no-expand-tabs (equivalent
>    to --expand-tabs=0) before or after the explicit --pretty=$fmt,
>    the tabs are expanded (or not expanded) accordingly.
>
> The tests use the second line of the log message for formats other
> than --pretty=short, primarily because the first line of the email
> format is handled specially to add the [PATCH] prefix, etc. in a
> separate codepath (--pretty=short uses the first line because there
> is no other line to test).
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/t/t4213-log-tabexpand.sh b/t/t4213-log-tabexpand.sh
> @@ -0,0 +1,98 @@
> +count_expand ()
> +{
> +       case " $* " in
> +       *' --pretty=short '*)
> +               line=$title ;;
> +       *)
> +               line=$body ;;
> +       esac
> +       expect=
> +       count=$(( $1 + $2 )) ;# expected spaces
> +       while test $count -gt 0
> +       do
> +               expect="$expect "
> +               count=$(( $count - 1 ))
> +       done
> +       shift 2
> +       count=$1 ;# expected tabs

Why semicolon before the hash here and above?

Re: [PATCH v5 4/4] pretty: test --expand-tabs

[Jeff King]

On Mon, Apr 04, 2016 at 09:10:46PM -0400, Eric Sunshine wrote:

> > +       count=$1 ;# expected tabs
> 
> Why semicolon before the hash here and above?

I am in the habit of doing this, too. I have a vague recollection of
getting bitten by a shell that treated:

  echo foo # bar

or something similar as not-a-comment. But neither bash, dash, nor ksh
seem to. So I'm not sure if it was some other shell in my past, or if I
simply have an irrational fear.

-Peff

Re: [PATCH v5 4/4] pretty: test --expand-tabs

[Jeff King]

On Mon, Apr 04, 2016 at 05:58:37PM -0700, Junio C Hamano wrote:

> +count_expand ()
> +{

This function takes a lot of unnamed arguments that we process with
"shift". It might be nice to give a brief comment describing them.

> +test_expand ""
> +test_expand --pretty
> +test_expand --pretty=short
> +test_expand --pretty=medium
> +test_expand --pretty=full
> +test_expand --pretty=fuller
> +test_expand --pretty=fuller
> +test_expand --pretty=raw
> +test_expand --pretty=email

Duplicated fuller?

-Peff

Re: [PATCH v5 0/4] Expanding tabs in "git log" output

[Jeff King]

On Mon, Apr 04, 2016 at 05:58:33PM -0700, Junio C Hamano wrote:

> So here is the fifth and hopefully the final try.  Previous round
> are at $gmane/289694, $gmane/289166, $gmane/288987 and
> $gmane/290222.

With the exception of two minor nits on the final patch, this looks good
to me. Thanks for cleaning up the option-parsing ordering thing; I think
it turned out rather nice.

-Peff

Re: [PATCH v5 4/4] pretty: test --expand-tabs

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Apr 04, 2016 at 09:10:46PM -0400, Eric Sunshine wrote:
>
>> > +       count=$1 ;# expected tabs
>> 
>> Why semicolon before the hash here and above?
>
> I am in the habit of doing this, too. I have a vague recollection of
> getting bitten by a shell that treated:
>
>   echo foo # bar
>
> or something similar as not-a-comment. But neither bash, dash, nor ksh
> seem to.

I think the reason why I started doing the same is because some
shells can be configured to lose the comment-introducer-ness of "#"
in interactive mode, and I wanted to make sure that many things I
write can be tried out by others more easily by copy-and-paste to
their interactive session.

Re: [PATCH v5 4/4] pretty: test --expand-tabs

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Apr 04, 2016 at 05:58:37PM -0700, Junio C Hamano wrote:
>
>> +count_expand ()
>> +{
>
> This function takes a lot of unnamed arguments that we process with
> "shift". It might be nice to give a brief comment describing them.
> ...
>> +test_expand --pretty=fuller
>> +test_expand --pretty=fuller
> ...
> Duplicated fuller?

Thanks.  Here is a replacement.

-- >8 --
The test prepares a simple commit with HT on its log message lines,
and makes sure that

 - formats that should or should not expand tabs by default do or do
   not expand tabs respectively,

 - with explicit --expand-tabs=<N> and short-hands --expand-tabs
   (equivalent to --expand-tabs=8) and --no-expand-tabs (equivalent
   to --expand-tabs=0) before or after the explicit --pretty=$fmt,
   the tabs are expanded (or not expanded) accordingly.

The tests use the second line of the log message for formats other
than --pretty=short, primarily because the first line of the email
format is handled specially to add the [PATCH] prefix, etc. in a
separate codepath (--pretty=short uses the first line because there
is no other line to test).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4213-log-tabexpand.sh | 105 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100755 t/t4213-log-tabexpand.sh

diff --git a/t/t4213-log-tabexpand.sh b/t/t4213-log-tabexpand.sh
new file mode 100755
index 0000000..e01a8f6
--- /dev/null
+++ b/t/t4213-log-tabexpand.sh
@@ -0,0 +1,105 @@
+#!/bin/sh
+
+test_description='log/show --expand-tabs'
+
+. ./test-lib.sh
+
+HT="	"
+title='tab indent at the beginning of the title line'
+body='tab indent on a line in the body'
+
+# usage: count_expand $indent $numSP $numHT @format_args
+count_expand ()
+{
+	expect=
+	count=$(( $1 + $2 )) ;# expected spaces
+	while test $count -gt 0
+	do
+		expect="$expect "
+		count=$(( $count - 1 ))
+	done
+	shift 2
+	count=$1 ;# expected tabs
+	while test $count -gt 0
+	do
+		expect="$expect$HT"
+		count=$(( $count - 1 ))
+	done
+	shift
+
+	# The remainder of the command line is "git show -s" options
+	case " $* " in
+	*' --pretty=short '*)
+		line=$title ;;
+	*)
+		line=$body ;;
+	esac
+
+	# Prefix the output with the command line arguments, and
+	# replace SP with a dot both in the expecte and actual output
+	# so that test_cmp would show the differene together with the
+	# breakage in a way easier to consume by the debugging user.
+	{
+		echo "git show -s $*"
+		echo "$expect$line"
+	} | sed -e 's/ /./g' >expect
+
+	{
+		echo "git show -s $*"
+		git show -s "$@" |
+		sed -n -e "/$line\$/p"
+	} | sed -e 's/ /./g' >actual
+
+	test_cmp expect actual
+}
+
+test_expand ()
+{
+	fmt=$1
+	case "$fmt" in
+	*=raw | *=short | *=email)
+		default="0 1" ;;
+	*)
+		default="8 0" ;;
+	esac
+	case "$fmt" in
+	*=email)
+		in=0 ;;
+	*)
+		in=4 ;;
+	esac
+	test_expect_success "expand/no-expand${fmt:+ for $fmt}" '
+		count_expand $in $default $fmt &&
+		count_expand $in 8 0 $fmt --expand-tabs &&
+		count_expand $in 8 0 --expand-tabs $fmt &&
+		count_expand $in 8 0 $fmt --expand-tabs=8 &&
+		count_expand $in 8 0 --expand-tabs=8 $fmt &&
+		count_expand $in 0 1 $fmt --no-expand-tabs &&
+		count_expand $in 0 1 --no-expand-tabs $fmt &&
+		count_expand $in 0 1 $fmt --expand-tabs=0 &&
+		count_expand $in 0 1 --expand-tabs=0 $fmt &&
+		count_expand $in 4 0 $fmt --expand-tabs=4 &&
+		count_expand $in 4 0 --expand-tabs=4 $fmt
+	'
+}
+
+test_expect_success 'setup' '
+	test_tick &&
+	sed -e "s/Q/$HT/g" <<-EOF >msg &&
+	Q$title
+
+	Q$body
+	EOF
+	git commit --allow-empty -F msg
+'
+
+test_expand ""
+test_expand --pretty
+test_expand --pretty=short
+test_expand --pretty=medium
+test_expand --pretty=full
+test_expand --pretty=fuller
+test_expand --pretty=raw
+test_expand --pretty=email
+
+test_done
-- 
2.8.1-253-gd0f4798

Re: [PATCH v5 4/4] pretty: test --expand-tabs

[Perry Hutchison]

Jeff King <peff@peff.net> wrote:
> > +test_expand --pretty=fuller
> > +test_expand --pretty=fuller
>
> Duplicated fuller?

Just brush it off :)





[Those too young to get the joke can
 look up "fuller brush" in Wikipedia.]