Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(css_formatter): support for pseudo selectors #1291

Merged
merged 1 commit into from
Dec 22, 2023

Conversation

faultyserver
Copy link
Contributor

Summary

#1285. This implements formatting support for (seemingly) all of the pseudo selector styles that the parser currently supports.

I'd like to deduplicate some of the work, specifically around how a lot of nodes implement the same parenthesize-and-indent pattern:

group(&format_args![
  l_paren_token.format(),
  soft_block_indent(the_value.format()),
  r_paren_token.format()
])

The consistency definitely feels like it should be managed in one place, but the difficulty is that some of the nodes use CssIdentifier while others have a specific SyntaxToken instead, so merging them isn't super straightforward. I might just make the block content re-usable? It's also a one time cost to write it all out, so maybe trying to unify them isn't super worth it.

Test Plan

Added a bunch of different spec tests to cover the various permutations of pseudo selectors.

Of note: the selector lists currently use join_with_*, which tries to respect newlines in the input source, but for CSS formatting we don't actually want to do that in most cases (all cases except rule list and declaration list). I'm going to leave that as something to tackle later, but it will be needed for Prettier compatibility in the future.

Copy link

netlify bot commented Dec 22, 2023

Deploy Preview for biomejs canceled.

Name Link
🔨 Latest commit 94db42f
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/6584fd3e4782230008d6039c

@github-actions github-actions bot added A-Formatter Area: formatter L-CSS Language: CSS labels Dec 22, 2023
@faultyserver faultyserver mentioned this pull request Dec 22, 2023
11 tasks
Copy link
Contributor

@denbezrukov denbezrukov left a comment

Choose a reason for hiding this comment

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

Great!
I've left some comments about differences with prettier outputs.

Comment on lines +86 to +92
:nth-child(
2n
+ 1 of li,

.test
) {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

is it expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is not expected! Thanks for catching it.

Comment on lines +34 to +43
:-webkit-any(i, p, :link, span:focus) {
}
:-webkit-any(
i,
p,
:link,

span:focus
) {
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the result of using the join_with_* builders right now, because they try to respect the newlines from the input source. It comes up in a few places where we want to just ignore the input and always output single newlines (like selector lists) or keep them flat (like this case).

I think I'll do a pass on all of these and fix them in one go in a follow up PR today.

Comment on lines +39 to +51
:lang(de, fr) {
}
:lang(
de,

fr
) {
}

:lang(de) {
}
:lang(de, fr, en, es, hi, pt) {
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as the above here, once the selector list stops trying to respect the newlines from the input, this should flatten out and match Prettier.

@faultyserver faultyserver merged commit bf1c3eb into main Dec 22, 2023
18 checks passed
@faultyserver faultyserver deleted the faulty/css-pseudo branch December 22, 2023 18:31
@Conaclos Conaclos added the A-Changelog Area: changelog label Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Formatter Area: formatter L-CSS Language: CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants