Re: [PATCH] pretty-print: de-tabify indented logs to make things line up properly
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 16, 2016, 21:55 UTC
- Message-ID
- <xmqq37rq3x5t.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <CA+55aFxLyMaur8Kd_uyewEeehPeuZXu=Hn11SSbTYd9O6FdDBA@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 8 quoted lines
> 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.
Show 42 quoted lines
> 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');
> }
> }