On Mon, Dec 19, 2022 at 6:00 PM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
> On Mon, Dec 19 2022, Lars Kellogg-Stedman wrote:
> > +test_expect_success 'setup tests for zero-width regular expressions' '
> > + cat >expect <<-EOF
> > + Modify func1() in file.c
> > + Add func1() and func2() in file.c
> > + EOF
> > +'
> > +
> > +test_expect_success 'zero-width regex $ matches any function name' '
> > + git log --format="%s" --no-patch "-L:$:file.c" >actual &&
> > + test_cmp expect actual
> > +'
>
> Untested, but as most of this is repeated & would fail if the "setup"
> test is skipped, a one-off function would be better, e.g.:
>
> test_zerowidth_regex () {
> local rx="$1" &&
> [...]
> }
> test_zerowidth_regex '$'
> test_zerowidth_regex '^'
> test_zerowidth_regex '.*'
>
> 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.
