Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:08:46
Linus Torvalds [off-list ref] 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.
quoted hunk
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'); } }