JS File Size Changes (gzipped)

A total of 1 files have changed, with a combined diff of -3 B (-0.8%).

Generated by 🚫 dangerJS against 004c996

This makes sense to me; do you think it's overkill to add tests for these small changes?

do you think it's overkill to add tests for these small changes?

Yes. The greedy: true is necessary to prevent a certain kind of behavior around greedy matching (see #2680 for more details). The problem is that this behavior is really hard to reproduce, so it's really hard to find a code snippet that could be the test.

I'm actually quite sure that it's not possible to find a code snippet that behaves differently under the new grammar in this PR. For the behavior described in #2680 to occur, we need a non-greedy pattern followed by at least 2 greedy patterns that can cause rematching amongst each other. The old nginx grammar simply doesn't fulfill those conditions.

The reason why I added greedy: true here and in many other places anyway is that greedy: true is guaranteed to prevent the issue described in #2680. It's a lot easier to just say "the first pattern has to be greedy if there are other greedy patterns" and then enforce that rule with an automated test (which I plan to add eventually) than it is to apply the complex reasoning around #2680 to every language we have.

Actually, I should probably make an issue for this practice and the test that will eventually enforce it.