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): Add quoteStyle for css formatting #1384

Merged
merged 5 commits into from
Dec 31, 2023

Conversation

faultyserver
Copy link
Contributor

Summary

#1285. This implements the quoteStyle option under the css.formatter options, allowing strings to be normalized to a single quote style, similar to how JS works.

Some additional notes:

  • Quote style is likely something that will be shared between a lot of languages. I moved the struct itself to the biome_formatter crate so that it could be reused, but I ran into quite a few issues when I tried to make it a global formatter option rather than a language-specific one. We're talking about overrides, and I think after we flesh that out it might be easier to do this. For now, it acts like a global formatter property since the CLI just uses --quote-style and applies to both instead of --*-formatter-quote-style for each, but it's not technically handled as top-level setting right now.
  • It'd be nice to move more of the logic for string normalization into the biome_formatter crate. Not much of that logic is really js-specific, but there is some stuff that can't be transferred. I just copied the implementation over and pared it down for CSS for now (and added the ability to add quotes to non-string tokens).
  • A lot of this PR is actually just cleanup of the settings for CSS (renamed from Json* things), and then even more of it is just wiring through the option from the CLI to the service to the formatter itself.

Test Plan

Added tests to the cli, service, and formatter to check that the config is being parsed correctly and the formatter respects the option when formatting strings.

Copy link

netlify bot commented Dec 31, 2023

Deploy Preview for biomejs canceled.

Name Link
🔨 Latest commit 2bba2c1
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/6591c86ee140190008ecaab0

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS labels Dec 31, 2023
@faultyserver faultyserver mentioned this pull request Dec 31, 2023
11 tasks
Copy link

codspeed-hq bot commented Dec 31, 2023

CodSpeed Performance Report

Merging #1384 will create unknown performance changes

⚠️ No base runs were found

Falling back to comparing faulty/css-quote-style (2bba2c1) with main (99ddb2d)

Summary

🆕 16 new benchmarks
⁉️ 32 dropped benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main faulty/css-quote-style Change
🆕 js_formatter[compiler.js] N/A 2.9 s N/A
🆕 js_formatter[d3.min.js] N/A 2.4 s N/A
🆕 js_formatter[vue.global.prod.js] N/A 1 s N/A
🆕 js_formatter[react-dom.production.min.js] N/A 801.2 ms N/A
🆕 js_formatter[jquery.min.js] N/A 674.5 ms N/A
🆕 js_formatter[pixi.min.js] N/A 2.7 s N/A
🆕 js_formatter[router.ts] N/A 37.3 ms N/A
🆕 js_formatter[parser.ts] N/A 117.4 ms N/A
🆕 js_formatter[checker.ts] N/A 4.9 s N/A
🆕 js_formatter[three.min.js] N/A 3.1 s N/A
🆕 js_formatter[react.production.min.js] N/A 40.5 ms N/A
🆕 js_formatter[dojo.js] N/A 161.2 ms N/A
🆕 js_formatter[ios.d.ts] N/A 4.3 s N/A
🆕 js_formatter[math.js] N/A 4.8 s N/A
🆕 js_formatter[typescript.js] N/A 18.9 s N/A
🆕 js_formatter[tex-chtml-full.js] N/A 5.9 s N/A
⁉️ jquery.min.js[uncached] 165 ms N/A N/A
⁉️ jquery.min.js[cached] 155.4 ms N/A N/A
⁉️ vue.global.prod.js[cached] 248.1 ms N/A N/A
⁉️ vue.global.prod.js[uncached] 267.9 ms N/A N/A
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@faultyserver faultyserver force-pushed the faulty/css-quote-style branch from ee796c2 to 10f5d9e Compare December 31, 2023 19:46
@faultyserver
Copy link
Contributor Author

i'm fairly positive the benchmark dropping/adding is just because it's stale, and the results on main haven't updated to the new grouping? gonna merge anyway since this change definitely doesn't touch those, so i don't believe it's related.

@faultyserver faultyserver merged commit 658ffe9 into main Dec 31, 2023
21 of 22 checks passed
@faultyserver faultyserver deleted the faulty/css-quote-style branch December 31, 2023 20:33
@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-CLI Area: CLI A-Formatter Area: formatter A-Project Area: project L-CSS Language: CSS L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants