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(biome_service,biome_cli): resolve .editorconfig files and merge configuration #2884

Merged
merged 1 commit into from
May 20, 2024

Conversation

dyc3
Copy link
Contributor

@dyc3 dyc3 commented May 16, 2024

Summary

This is part of a series of PRs to add .editorconfig support to biome. As with the previous PR, this is intentionally small to help make code reviews easier. The goal is to add functions that look for and load the .editorconfig files, and merge the loaded configurations.

Here's how the configurations are prioritized:

  • cli args overrides biome.json
  • biome.json overrides .editorconfig
  • .editorconfig overrides biome defaults

Related to: #1724

Test Plan

I've added one test just to show that it works in this PR, but I have more tests in another branch that I could bring into this PR instead of making another one.

cargo test -p biome_service
cargo test -p biome_cli

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project labels May 16, 2024
@dyc3 dyc3 force-pushed the 05-13-editorconfig_search_for_and_load branch from c1a2d1c to 9e52d18 Compare May 16, 2024 11:47
@dyc3 dyc3 marked this pull request as ready for review May 16, 2024 13:16
@dyc3 dyc3 force-pushed the 05-13-editorconfig_search_for_and_load branch from 9e52d18 to 29a3a97 Compare May 16, 2024 15:54
crates/biome_service/src/configuration.rs Outdated Show resolved Hide resolved
crates/biome_service/src/configuration.rs Outdated Show resolved Hide resolved
@dyc3 dyc3 force-pushed the 05-13-editorconfig_search_for_and_load branch 2 times, most recently from 8c60319 to f9ccaa5 Compare May 17, 2024 19:29
@dyc3 dyc3 requested review from Sec-ant and ematipico May 17, 2024 19:29
crates/biome_cli/tests/cases/editorconfig.rs Outdated Show resolved Hide resolved
crates/biome_cli/tests/cases/editorconfig.rs Outdated Show resolved Hide resolved
crates/biome_cli/src/commands/check.rs Show resolved Hide resolved
crates/biome_cli/src/commands/check.rs Outdated Show resolved Hide resolved
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

I left some suggestions and changes to improve the quality of the PR. We are almost there, we just need to add more test cases to cover the new errors

crates/biome_configuration/src/editorconfig.rs Outdated Show resolved Hide resolved
crates/biome_configuration/src/editorconfig.rs Outdated Show resolved Hide resolved
crates/biome_cli/src/commands/check.rs Show resolved Hide resolved
crates/biome_cli/src/commands/check.rs Outdated Show resolved Hide resolved
crates/biome_cli/src/commands/format.rs Outdated Show resolved Hide resolved
crates/biome_configuration/src/editorconfig.rs Outdated Show resolved Hide resolved
crates/biome_configuration/src/editorconfig.rs Outdated Show resolved Hide resolved
crates/biome_service/src/configuration.rs Show resolved Hide resolved
crates/biome_service/src/configuration.rs Outdated Show resolved Hide resolved
crates/biome_service/src/configuration.rs Outdated Show resolved Hide resolved
@dyc3 dyc3 mentioned this pull request May 18, 2024
@dyc3 dyc3 force-pushed the 05-13-editorconfig_search_for_and_load branch from f9ccaa5 to 0d8c22b Compare May 18, 2024 13:44
@github-actions github-actions bot added the A-Diagnostic Area: diagnostocis label May 18, 2024
Copy link

codspeed-hq bot commented May 18, 2024

CodSpeed Performance Report

Merging #2884 will not alter performance

Comparing dyc3:05-13-editorconfig_search_for_and_load (c8455ae) with main (9a05b77)

Summary

✅ 92 untouched benchmarks

@dyc3 dyc3 force-pushed the 05-13-editorconfig_search_for_and_load branch from 0d8c22b to 5e1b6f8 Compare May 18, 2024 15:04
@dyc3 dyc3 requested review from Sec-ant and ematipico May 18, 2024 15:05
Copy link
Member

@Sec-ant Sec-ant left a comment

Choose a reason for hiding this comment

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

All my concerns are addressed, thank you ❤️

crates/biome_configuration/src/editorconfig.rs Outdated Show resolved Hide resolved
crates/biome_service/src/diagnostics.rs Outdated Show resolved Hide resolved
@dyc3 dyc3 force-pushed the 05-13-editorconfig_search_for_and_load branch from 5e1b6f8 to ab79531 Compare May 18, 2024 16:18
crates/biome_cli/src/commands/check.rs Outdated Show resolved Hide resolved
crates/biome_cli/src/commands/check.rs Outdated Show resolved Hide resolved
crates/biome_cli/src/commands/format.rs Outdated Show resolved Hide resolved
crates/biome_cli/src/commands/format.rs Outdated Show resolved Hide resolved
crates/biome_cli/tests/cases/editorconfig.rs Show resolved Hide resolved
crates/biome_diagnostics/src/adapters.rs Outdated Show resolved Hide resolved
@dyc3 dyc3 force-pushed the 05-13-editorconfig_search_for_and_load branch from ab79531 to c8455ae Compare May 19, 2024 12:54
@dyc3 dyc3 requested a review from ematipico May 19, 2024 12:55
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time for addressing my pedantic concerns, that's really appreciated!

@ematipico ematipico merged commit e848ee1 into biomejs:main May 20, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Diagnostic Area: diagnostocis A-Project Area: project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants