Re: [PATCH v2 3/4] add -p: handle `diff-so-fancy`'s hunk headers better

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 29, 2022, 17:19 UTC
Message-ID: <xmqqilmbotl0.fsf@gitster.g>
In-Reply-To: <0q0psp09-8993-96r6-3r90-q4s368p98510@tzk.qr>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 8 quoted lines

> Here is why: in the _regular_ case, i.e. when we have a colored hunk
> header like `@@ -936 +936 @@ wow()`, we manage to parse the line range,
> and then record the offset of the extra part that starts afterwards.
>
> This extra part is non-empty, therefore we add an extra space.
>
> But that part already starts with a space, so now we end up with two
> spaces.

In other words, this breaks because the original code depended on having the extra whitespace before the "funcname" part.

Stepping back a bit, if the final goal for the UI generation out of this string is to append the material with a whitespace before it for better visual separation, then the original should probably have (at least conceptually) lstrip'ed the leading whitespaces from the string it found after " @@" and then appended the result to where it is showing, with its own single whitespace as a prefix. It would have prevented this bug from happening by future-proofing, and made the final output nicer, as any excess whitespaces between the " @@" and "funcname" would have been turned into a single SP.

The "prepend one iff it does not already begin with a whitespace" is a (at least mentally to the developer) less expensive approach that is fine, too.

Thanks.