Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> 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');
> }
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html