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

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Mar 16, 2016, 19:50 UTC
Message-ID: <CA+55aFxLyMaur8Kd_uyewEeehPeuZXu=Hn11SSbTYd9O6FdDBA@mail.gmail.com>
In-Reply-To: <xmqqwpp243sb.fsf@gitster.mtv.corp.google.com>

On Wed, Mar 16, 2016 at 12:32 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 5 quoted lines

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

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