Re: [PATCH v4] line-range: fix infinite loop bug with '$' regex

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 20, 2022, 01:07 UTC
Message-ID: <xmqqo7ryzxu8.fsf@gitster.g>
In-Reply-To: <CAPig+cSEtW3TpUkQhNVjM=7WBk-3ZYOCDohsu+AZSpEAPKwk2w@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 9 quoted lines

>> The change of direction for the fix itself looks good to me (re my
>> earlier feedback on a previous round), i.e. that we should fix our own
>> code, not forbid '$' in regexes.
>
> It's subjective, of course, but the patch seems "good enough" as-is
> and the tests are easy to understand. Therefore, can you clarify for
> Lars and other reviewers if you're merely presenting an alternative
> approach, or if you consider your suggestion "blocking" and expect a
> reroll.

Thanks for saying this.

I personally felt that the original was easier to follow---given that these tests do not have too many moving parts that may need to be changed (in which case abstracting common parts out would allow us to maintain only one copy which is a big win), being able to read from top to bottom without having to read a function and recall what the parameter was etc.