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): handle unset and off values in editorconfig files #3907

Merged

Conversation

dyc3
Copy link
Contributor

@dyc3 dyc3 commented Sep 15, 2024

Summary

In the editorconfig spec, unset means "use the default value" and off (in the context of max_line_length) means "completely turn off the line limit".

This PR aims to implement the behavior for unset, and off. provide a better diagnostic when encountering off. We technically can't really handle off because biome doesn't have a mechanism to turn off the max line length enforcement.

fixes #3886
closes #3904

Test Plan

Added some unit tests and a cli snapshot test

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project labels Sep 15, 2024
@dyc3 dyc3 requested review from a team September 15, 2024 22:10
@dyc3 dyc3 force-pushed the 09-15-fix_config_handle_unset_and_off_values_in_editorconfig_files branch from c223874 to f0e2039 Compare September 15, 2024 22:53
```block
configuration ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× Key 'max_line_length' is incompatible with biome: 'off' is incompatible with how Biome works.
Copy link
Member

Choose a reason for hiding this comment

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

The spec says that off means "use the default settings". Can't we do that?

Regarding the message, I think we could be more specific. Saying "incompatible with Biome" seems to be too generic. Also, the message isn't actionable, because it doesn't tell the user what they should do to fix the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah we can do that.

Btw, the only mention of "off" seems to be in this document: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties and it doesn't seem to be present in https://spec.editorconfig.org/

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I noticed that too 🥲

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps they standardized off to unset?

@dyc3 dyc3 force-pushed the 09-15-fix_config_handle_unset_and_off_values_in_editorconfig_files branch from f0e2039 to ca791ca Compare September 16, 2024 04:03
@dyc3 dyc3 force-pushed the 09-15-fix_config_handle_unset_and_off_values_in_editorconfig_files branch from ca791ca to 045f3da Compare September 16, 2024 12:06
@dyc3 dyc3 merged commit 2a775c7 into main Sep 16, 2024
13 checks passed
@dyc3 dyc3 deleted the 09-15-fix_config_handle_unset_and_off_values_in_editorconfig_files branch September 16, 2024 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Project Area: project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Editorconfig fails to parse max_line_length = off 🐛A unset value in editorconfig is not supported.
3 participants