Ensure strings are consumed as-is when using internal segment()

[RobinMalfait]

This PR fixes an issue where parens, bracket and curlies needed to be balanced inside of strings instead of consuming the string until the end.

When encountering strings when using segment we didn't really treat them as actual strings. This means that if you used any parens, brackets, or curlies then we wanted them to be properly balanced.

This should not be the case, whenever we encounter a string, we want to consume it as-is and don't want to worry about bracket balancing. We will now consume it until the end of the string (and make sure that escaped closing quotes are not seen as real closing quotes).

Fixes: #13607

[thecrypticace]

LGTM — just one tweak in the tests plz

[brandonmcconnell]

@RobinMalfait @thecrypticace It may be useful to include tests for bracket and quote chars in arbitrary values, specifically, to include tests similar to the original issue: e.g. supports-[content:"("]:[content:"("]

We don't need to include all of these, but these should all pass with the proper fix, perhaps with some minor alterations: https://play.tailwindcss.com/EAhSfQFfTa

[adamwathan]

Nice! This looks good to me.

Couple things:

• Should we add a test somewhere for the higher level issue that made us notice this, or is there not really a good/obvious place to put it? It's kinda weird to put it in the supports tests because it applies to other stuff like the data variant too.
• Did you benchmark this against the previous implementation (even though it had a bug)? Hoping it's basically negligible.

[brandonmcconnell]

@RobinMalfait Thanks for the quick fix here!

[RobinMalfait]

Yeah good ideas to add another, more high level test.

My thinking is to add a generic test for candidate parsing so that we know that we at least parse it into the correct data structure. If we want, we can add a specific one for supports-[...] but as you mentioned, this is a more generic fix anyway.

Right now we don't really have generic tests for this.

Benchmarking wise, it's actually even better when we encounter strings because we can skip a lot of work. The checks to know that we are in a string or a single integer check.

Current PR:

 ✓ src/utils/segment.bench.ts (1) 792ms
     name               hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · segment  1,087,386.63  0.0008  0.9782  0.0009  0.0009  0.0010  0.0012  0.0033  ±0.53%   543694   fastest

Next branch:

 ✓ src/utils/segment.bench.ts (1) 789ms
     name               hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · segment  1,042,913.63  0.0008  0.9799  0.0010  0.0010  0.0011  0.0014  0.0028  ±0.55%   521457   fastest

[adamwathan]

Should we change this to represent the user-facing fix? Maybe something more like "Support unbalanced parentheses and braces in quotes in arbitrary values and variants".

[adamwathan]

This might look less weird with the comment above the while instead of in the condition.