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

fix(config): default value for indent_width #3096

Merged
merged 5 commits into from
Jun 6, 2024
Merged

Conversation

ematipico
Copy link
Member

@ematipico ematipico commented Jun 6, 2024

Summary

The issue is that we had IndentWidth, but we weren't using it at all 😆

Now we use it, and I took the chance to add a max value that we cant' exceed. It behaves the same way LineWidth does.

Closes #3067

Test Plan

I added a new test case

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project A-Formatter Area: formatter A-Changelog Area: changelog labels Jun 6, 2024
@ematipico ematipico requested review from a team June 6, 2024 13:49
@github-actions github-actions bot added L-JavaScript Language: JavaScript and super languages L-CSS Language: CSS L-JSON Language: JSON and super languages labels Jun 6, 2024
Copy link

codspeed-hq bot commented Jun 6, 2024

CodSpeed Performance Report

Merging #3096 will degrade performances by 7.14%

Comparing fix/regression-staged (e8354ed) with fix/regression-staged (a3cbb47)

Summary

❌ 2 regressions
✅ 90 untouched benchmarks

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

Benchmarks breakdown

Benchmark fix/regression-staged fix/regression-staged Change
pure.css[cached] 3.7 ms 4 ms -7.14%
jquery.min.js[cached] 26.9 ms 28.8 ms -6.41%

crates/biome_formatter/src/lib.rs Outdated Show resolved Hide resolved
crates/biome_formatter/src/lib.rs Outdated Show resolved Hide resolved
crates/biome_formatter/src/lib.rs Outdated Show resolved Hide resolved
@ematipico ematipico requested review from Conaclos and Sec-ant June 6, 2024 15:06
@ematipico ematipico force-pushed the fix/regression-staged branch from 1647647 to 8ae7a3d Compare June 6, 2024 15:07
@ematipico ematipico force-pushed the fix/regression-staged branch from 8ae7a3d to 493f2c1 Compare June 6, 2024 15:14
@ematipico ematipico requested a review from Sec-ant June 6, 2024 15:14
@ematipico ematipico force-pushed the fix/regression-staged branch from 493f2c1 to e8354ed Compare June 6, 2024 15:58
@ematipico ematipico merged commit 7c83e79 into main Jun 6, 2024
12 of 14 checks passed
@ematipico ematipico deleted the fix/regression-staged branch June 6, 2024 16:09
fn default_css() {
let css_configuration = CssFormatter::default();

assert_eq!(css_configuration.indent_width, IndentWidth::default());
Copy link
Contributor

Choose a reason for hiding this comment

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

The default value should be None; otherwise, the CSS formatter will not inherit indent_width from the global formatter settings when css.formatter.indent_width is unset.

Related: #3103

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 L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📝 CSS formatter indentWidth defaults to 0 (not 2 as intended) when using spaces
4 participants