Allow pseudo classes in nested tcss

[rodrigogiraoserrao]

This tweaks the tokenizer to avoid parsing strings like "str1:str2" as rule declarations if str2 is a valid pseudo-class and we're in a nested context.

Fixed #4039.

This PR depends on #4040.

This PR is also a compromise that handles the 99.9% case. If someone uses a valid pseudo-class as the value for the rules layer or layers in a nested context, the tokenizer will try to parse that as a selector + pseudo-class and that will fail.

We use a negative lookahead in the regex for the declaration instead of a positive lookahead in the regex for the pseudo-class because that's what yields the better error message when the user writes something that looks like a pseudo-class but that's misspelled.

Other alternatives include modifying the tokenizer to backtrack and try alternative tokenization routes (doesn't sound very feasible since we're yielding tokens which are consumed immediately) and expanding the number of states the tokenizer recognises to create a “path” that disambiguates between rule + value or selector + pseudo-class when it finds a ; or not.

[rodrigogiraoserrao]

@willmcgugan that does not generate a helpful “did you mean” error. It didn't before the PR and still doesn't after the PR.

You get that error because of the ambiguity of this situation. The tokenizer has to try the "rule:value" path first and because "locus" is not a valid pseudoclass, the negative lookahead won't prevent it from thinking it's a declaration. It will then be mad at you when it finds the {.

[rodrigogiraoserrao]

P.S. it will if you add the nested & because that removes the ambiguity, which is pretty much what you mentioned when we talked earlier.

[willmcgugan]

It will give an error if its not nested.

What error does your change give you with this CSS ?

Screen {
    Label:locus {
        border red;
    }
}

[rodrigogiraoserrao]

This is the error you get before and after the PR. 👇 I know we are able to generate a more helpful “did you mean” error when the TCSS is not nested.

I can't add the helpful error message in this case without either:

• changing the tokenizer significantly to remove the ambiguity of parsing this nested TCSS; or
• breaking helpful error messages for rule names (e.g., when you do baground: red it asks if you meant background: red).

[willmcgugan]

I think we can do better. I think we can resolve the ambiguity while maintaining helpful error messages.

We might be able to exploit the fact that there are a limited set of rule names. i.e. in the case above Label: is clearly not a declaration because there is no Label rule.

Up for the challenge? @rodrigogiraoserrao

[rodrigogiraoserrao]

@willmcgugan I've updated the top comment to describe what I'm doing and show some of the error messages we get. Images 2-6 are probably what you expect and image 1 (top-left) is probably the most unexpected one.

On the one hand, for a Human reading it it's “obvious” that the dev was trying to define a nested selector + pseudo-class, even though “bananas” is a ridiculous pseudo-class. On the other hand, I don't see how to determine this with regex.

I thought of looking at the end of the line:

• if the line ends with { it's probably a selector + pseudo-class
• if the line ends with ; it's probably a rule + value

But then I'd also need to check for comments (multi-line and single line), check if there were loads of CSS inlined or not, etc. This feels like an impossible task for regex.

[willmcgugan]

I think we can do better than "Invalid CSS".

What if we were to simply mandate that types begin with a capital letter? If that were the case, there would be no ambiguity with a declaration versus a selector.

[rodrigogiraoserrao]

I'm assuming all future declarations would then start with a lower-case letter. That looks promising. Should I go down that route?

[willmcgugan]

Declarations will always be lower case. Go for it!

[rodrigogiraoserrao]

Superseded by #4252 given that the trade-off suggested by Will simplifies the implementation. I created a new PR because I don't think there's any reason to leave all these commits and reverts in the history of the PR that gets merged.