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

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- 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');
 	}
 }