Skip to content

Fix CSS Nesting selector validation#47

Open
torresgol10 wants to merge 33 commits intomicrosoft:mainfrom
torresgol10:fix/css-nesting
Open

Fix CSS Nesting selector validation#47
torresgol10 wants to merge 33 commits intomicrosoft:mainfrom
torresgol10:fix/css-nesting

Conversation

@torresgol10
Copy link
Copy Markdown

@torresgol10 torresgol10 commented Nov 21, 2025

Supersedes #30. Continues the work of @jacobcassidy to add CSS Nesting support.

This should fix #9 and #15.

Changes:

Updated css.cson to allow & followed by identifier characters.
Added tests in css-spec.mjs covering nesting scenarios.

Copilot AI review requested due to automatic review settings November 21, 2025 10:16
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds CSS Nesting support by fixing the validation of the nesting selector & when followed immediately by other selector parts (e.g., &:hover, &.foo). The grammar now correctly recognizes & as a valid nesting selector and includes it in selector matching patterns throughout the grammar.

Key Changes:

  • Added nesting selector & as a valid selector type with scope entity.name.tag.nesting.css
  • Updated selector validation patterns to include & in lookahead and lookaround assertions
  • Added comprehensive test coverage for CSS nesting scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
spec/css-spec.mjs Adds four comprehensive test cases covering the nesting selector & in isolation, with classes, with pseudo-classes, and with combinators
grammars/css.cson Updates the CSS grammar to recognize & as a valid nesting selector, refactors arithmetic operators, adds function nesting support, and updates selector validation patterns to include & in various contexts

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}
{
'include': '#property-values'
},
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing comma after the object in the patterns array. This is invalid CSON syntax and will cause a parsing error.

Suggested change
},
}

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@torresgol10
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@thernstig
Copy link
Copy Markdown

@torresgol10 does this need to be fixed here?

jacobcassidy/vscode-css-nesting-syntax-highlighting#8

@torresgol10
Copy link
Copy Markdown
Author

Hi @thernstig , thanks for linking that issue! It's an interesting topic.

However, this PR won't fix that problem. The directives mentioned (@variant, @utilities, @apply, etc.) are Tailwind-specific syntax, not standard CSS. This PR focuses solely on native CSS Nesting support as per the W3C specification.

@Athari
Copy link
Copy Markdown

Athari commented Dec 31, 2025

Is this fix to multi-line urls #28 (comment) not integrated into this PR?

				{
					"begin": "\\\\$\\s*?",
					"end": "^(?<!\\G)",
					"name": "constant.character.escape.newline.css"
				},

Considering how long it's taking to get this merged, let's not forget about that tiny thing which completely breaks highlighting in a whole file. 😆

@controversial
Copy link
Copy Markdown

That seems unrelated to this PR

@Athari
Copy link
Copy Markdown

Athari commented Jan 4, 2026

@controversial
It's a trivial single-character fix which the original author of this PR @jacobcassidy intended to include but got distracted with life.

If this single character actually breaks tests or something, I can understand moving it to a separate PR, but if it doesn't, that would seem to be a waste of resources.

@thernstig
Copy link
Copy Markdown

@aeschli are you the one to review this?

@aeschli
Copy link
Copy Markdown
Contributor

aeschli commented Mar 31, 2026

@torresgol10 Is the PR ready? How confident are you it does not break the existing highlighter?

@alexr00 FYI

@romainmenke
Copy link
Copy Markdown
Contributor

I went over this PR a while back and it seems correct except for a few extreme edge cases.

CSS nesting doesn't require a leading & or other symbol to announce that a selector was started. This implies that the syntax clashes with regular declarations. See: #30 (comment)

The mentioned examples in that comment are now handled, but in a way that is maybe a bit too specialized? I think this is fine :)

I couldn't find regressions introduced by this change and I think that any edge cases that remain can be deferred for now.

Looks like solid work by everyone involved, especially @jacobcassidy and @torresgol10 🙇

@torresgol10
Copy link
Copy Markdown
Author

torresgol10 commented Mar 31, 2026

@aeschli The PR is ready and fully tested. Here's a summary:

Test results:

  • All 207 existing tests pass (0 failures, 6 pre-existing skips)
  • 5 new CSS Nesting tests added and passing
  • 4 new tests for advanced property values (if(), custom functions --name(), JSON-like structures in custom properties)
  • CSON grammar validates correctly (34 repository rules, 143 #include references — all resolved)
  • Additional edge-case verification on 18+ common CSS patterns — no regressions found

Regarding issue #28 (multiline string \ breaking entire file highlighting): the fix is a single-character change ([\s*] → [\s*?] but is unrelated to CSS Nesting. It will be submitted in a separate PR to keep this one focused.

Confident this doesn't break the existing highlighter.

@thernstig
Copy link
Copy Markdown

Just a note that I think this will close many other issues:

I did not verify this or do a deep dive, but some comments indicate this. Might be worth to check various issues once this is merged.

@torresgol10
Copy link
Copy Markdown
Author

torresgol10 commented Apr 1, 2026

@thernstig Good catch! I verified all three issues:

Here's how it looks with this PR applied:

image

If anything doesn't look right, let me know and I can adjust.

A note on the red underlines (validation errors): Some newer CSS syntax like if() may still show validation warnings/errors (red underlines). This is not caused by this PR or this grammar — it comes from the vscode-css-languageservice, which is a separate repository. VS Code currently bundles an older version of that service that doesn't recognize some newer CSS features yet. The newer version of vscode-css-languageservice already has updates that address some of these, but they haven't been shipped in VS Code yet.

@thernstig
Copy link
Copy Markdown

thernstig commented Apr 1, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for nested CSS

8 participants