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

📎 .editorconfig support #1724

Closed
ematipico opened this issue Feb 1, 2024 · 29 comments · Fixed by #3343
Closed

📎 .editorconfig support #1724

ematipico opened this issue Feb 1, 2024 · 29 comments · Fixed by #3343
Assignees
Labels
A-Formatter Area: formatter S-Enhancement Status: Improve an existing feature S-Feature Status: new feature to implement S-Funding Status: open to funding and implemented by external contributors S-Help-wanted Status: you're familiar with the code base and want to help the project

Comments

@ematipico
Copy link
Member

ematipico commented Feb 1, 2024

Description

We would like to support the .editorconfig file.

Here's some thoughts about the feature:

  • the file .editorconfig should have higher priority over biome.json, but less priority over the CLI options
  • we could use https://docs.rs/rust-ini/latest/ini/ to parse the file
  • in a CLI setting, we should start looking for the file from the working directory, and use the auto_search to query the parent folders until we find one
    • biome.json and .editorconfig can be a different folders, so their resolution should be separate
  • in a LSP setting, we should start looking for the file fro mthe root directory of the project. and then use auto_search to query the parent folders until we find one
    • biome.json and .editorconfig can be a different folders, so their resolution should be separate
  • should this feature be opt-in or opt-out?

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@ematipico ematipico added S-Help-wanted Status: you're familiar with the code base and want to help the project A-Formatter Area: formatter S-Enhancement Status: Improve an existing feature labels Feb 1, 2024
@ematipico ematipico changed the title 📎 .editoconfig support 📎 .editorconfig support Feb 1, 2024
@Conaclos
Copy link
Member

Conaclos commented Feb 1, 2024

the file .editorconfig should have higher priority over biome.json

I think this should be the reverse as prettier is doing. We should certainly convert internally the .editorconfig to a biome config and merge bione.json inside it?

@ematipico
Copy link
Member Author

I thought they were doing the opposite. Sounds good to me

@ematipico ematipico added the S-Feature Status: new feature to implement label Feb 5, 2024
@miafoo
Copy link

miafoo commented Feb 10, 2024

I just ran into this "problem" on my own project and it sounds like a great idea. I feel like it should be using .editorconfig rules by default, and anything you explicitly define in your biome.json file should override it and be opt-out by default.

@Deide
Copy link

Deide commented Feb 23, 2024

Yep, fantomas (for F#) has their config values inside .editorconfig too, and it's quite nice for those overlapping areas of concern.

@polar-sh polar-sh bot added the S-Funding Status: open to funding and implemented by external contributors label Feb 23, 2024
@kyr0
Copy link

kyr0 commented Feb 25, 2024

Yep, would be great if biome would read the .editorconfig and use it config, overriding the defaults of biome. But when the user is explicitly setting values in biome config, biome config values should win. This way, it would work for any project using editorconfig from the start, and still not cause regressions for any project not using editorconfig (but maybe having it still in the repo)

Schematically:

=> biome user settings win over
=> .editorconfig values win over
=> biome defaults

Might need deep-merges if we have section support for cases like:

# editorconfig.org

root = true

[*]
indent_style = space
indent_size = 4
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true

[*.{tsx}]
indent_size = 2
trim_trailing_whitespace = false

thedannywahl added a commit to thedannywahl/inst-app-template that referenced this issue Apr 10, 2024
align biome config to editorconfig until biome supports editorconfig biomejs/biome#1724
@dyc3
Copy link
Contributor

dyc3 commented May 13, 2024

I would like to pick this up.

It looks like prettier has a separate config option to enable support for .editorconfig: https://prettier.io/docs/en/configuration.html#editorconfig Do we want to match that? Or just support it automagically?

@ematipico
Copy link
Member Author

I would like to pick this up.

It looks like prettier has a separate config option to enable support for .editorconfig: prettier.io/docs/en/configuration.html#editorconfig Do we want to match that? Or just support it automagically?

I believe that option is set to true by default. I suppose this option should be set to true for Biome too.

@dyc3
Copy link
Contributor

dyc3 commented May 14, 2024

A quick status update: I have a working prototype, so I'm going to start cleaning it up a bit and submitting PRs today.

@dyc3
Copy link
Contributor

dyc3 commented May 18, 2024

Todo list of remaining tasks, mostly for myself.

Copy link
Member Author

Thank you @dyc3 for contributing to close this issue! ⭐

The rewards from this issue, totaling $50, has been shared with you.

What now?

  1. Create a Polar account
  2. See incoming rewards & setup Stripe to receive them
  3. Get payouts as backers finalize their payments

If you already have a Polar account setup, you don't need to do anything.

@kyr0
Copy link

kyr0 commented Jul 6, 2024

Well done, @dyc3 ! Thank you :)

@junaga
Copy link

junaga commented Jul 16, 2024

lfg, the future looks bright. imho bimo should also show some kind of warning if .editorconfig and biome options are both available. it should either be one or the other. checking a repo into biome should mean a configuration rewrite, from .editorconfig to biome config.

@junaga
Copy link

junaga commented Jul 16, 2024

oh and i think the docs need a fixup

@dyc3
Copy link
Contributor

dyc3 commented Jul 16, 2024

warning if .editorconfig and biome options are both available.

Do you mean if they have conflicting options? I'm a little confused on what you're looking for.

@nemchik
Copy link

nemchik commented Jul 16, 2024

I actually like the idea of there being a warning (not error) if there is a conflict between .editorconfig and biome.json

@ematipico
Copy link
Member Author

warning if .editorconfig and biome options are both available.

Do you mean if they have conflicting options? I'm a little confused on what you're looking for.

I think they mean in case the same option is specified in both places

@ematipico
Copy link
Member Author

oh and i think the docs need a fixup

The feature isn't available yet, officially. I'm sure @dyc3 will send a PR to update the documentation when we're close to the release

@nemchik
Copy link

nemchik commented Jul 16, 2024

warning if .editorconfig and biome options are both available.

Do you mean if they have conflicting options? I'm a little confused on what you're looking for.

I think they mean in case the same option is specified in both places

Adding to this;

In case the same option is specified in both places with non equivalent values.

If the values in both places are equivalent I don't think it's worth a warning (maybe still worth logging as something other than warning/error). Also, since there is a documented priority, I don't think a conflict should be an error (ex: errors can fail CI or pre-commit) but a warning makes sense to me.

@ematipico
Copy link
Member Author

I'm not really sure we should have any kind of warning, in any case.

The .editorconfig file can also be placed in places outside of the project. A user might want to use a certain set of options for ALL their projects. But then, each project can have slightly different options (for example, a work-related project VS a personal project).

It doesn't make much sense to have a warning in case that's the expected behaviour of the user. Plus, it's possible to fail CI with warnings with a certain option.

It could make sense to provide, maybe a verbose, info diagnostic.

@junaga
Copy link

junaga commented Jul 17, 2024

monorepo support without explicit opt-in?

@ematipico
Copy link
Member Author

monorepo support without explicit opt-in?

I am not really sure what you mean, but .editorconfig, usually, is unrelated to a monorepo. .editorconfig is usually a file placed at the root of a generic repository. Having a .editorconfig file inside each lib isn't the usual setup

@junaga
Copy link

junaga commented Jul 17, 2024

sorry i have no idea. i thought you were suggesting to have biome config nested in a .editorconfig workspace. are there really sensible situations where a project should have both configuration files set?

the way i see it biome is a different tool to opt into, a better one. I understand the paradigm of progressive enhancement - for supporting editor application here. i'm just asking if it makes sense here.

@ematipico
Copy link
Member Author

ematipico commented Jul 17, 2024

are there really sensible situations where a project should have both configuration files set?

Yes, there are! This very repository is an example. You might have files that Biome can't/doesn't handle, but the editor used by a company can handle them, and format them. In the wild, there might be some esoteric projects where you might have some expected files, e.g.: .ini, extensionless files e.g. makefile, .gitignore and more

@junaga
Copy link

junaga commented Jul 18, 2024

ah, okay that makes perfect sense, i just never formated those so far. thanks for the reply

@difosfor
Copy link

So is the Biome formatter supposed to completely support .editorconfig? Because it seems like it does not support trim_trailing_whitespace = true. And it also doesn't seem to pick up on the indent_style = space in mine. Should I create an issue?

@dyc3
Copy link
Contributor

dyc3 commented Oct 11, 2024

Editorconfig support is not enabled by default until 2.0. I think trim_trailing_whitespace should already be handled by the formatter. Feel free to file an issue with a repro.

@difosfor
Copy link

difosfor commented Oct 11, 2024

I enabled it in the config under formatter using "useEditorconfig": true, but it doesn't seem to work. I'll double check and create an issue.

@jovand3v
Copy link

jovand3v commented Dec 7, 2024

@difosfor Experiencing the same issue, can't get the indent_style = space to work at all. Getting ignored both in biome.json and in .editorconfig.

@dyc3
Copy link
Contributor

dyc3 commented Dec 7, 2024

@jovand3v Could you open a new issue with a reproduction? That would help a ton.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Formatter Area: formatter S-Enhancement Status: Improve an existing feature S-Feature Status: new feature to implement S-Funding Status: open to funding and implemented by external contributors S-Help-wanted Status: you're familiar with the code base and want to help the project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants