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, 19:47 UTC
Message-ID: <xmqqoaae4340.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqwpp243sb.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

> The above is most likely what I would have written if I were doing
> this patch.  I could squash it to save a round-trip, but let me run
> the testsuite first to see if we need adjustments to existing tests.

Strangely running t4201 with your patch (without any squashing) seems to show a breakage in shortlog. I won't be able to come back to this topic for at least a few hours, so this is just a single bit "breaks" report, without "how and why" analysis, sorry.

Show 30 quoted lines

>
> Also your idea:
>
>> But the code *could* be made to just always do the whole
>> "strbuf_add()", and not return a return value at all, and the no-tab
>> case wouldn't be explicitly written to be different.
>
> 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?
>
> @@ -1723,10 +1711,14 @@ void pp_remainder(struct pretty_print_context *pp,
>  
>  		strbuf_grow(sb, linelen + indent + 20);
>  		if (indent) {
> -			if (pp_handle_indent(sb, indent, line, linelen))
> -				linelen = 0;
> +			strbuf_addchars(sb, ' ', indent);
> +			if (pp->fmt == CMIT_FMT_EXPAND_TABS)
> +				strbuf_expand_add(sb, line, linelen);
> +			else
> +				strbuf_add(sb, line, linelen);
> +		} else {
> +			strbuf_add(sb, line, linelen);
>  		}
> -		strbuf_add(sb, line, linelen);
>  		strbuf_addch(sb, '\n');
>  	}
>  }