biome icon indicating copy to clipboard operation
biome copied to clipboard

📎 `.editorconfig` support

Open ematipico opened this issue 1 year ago • 9 comments

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 avatar Feb 01 '24 11:02 ematipico

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?

Conaclos avatar Feb 01 '24 11:02 Conaclos

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

ematipico avatar Feb 01 '24 12:02 ematipico

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.

miafoo avatar Feb 10 '24 07:02 miafoo

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

Deide avatar Feb 23 '24 15:02 Deide

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

kyr0 avatar Feb 25 '24 13:02 kyr0

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?

dyc3 avatar May 13 '24 11:05 dyc3

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.

ematipico avatar May 13 '24 12:05 ematipico

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 avatar May 14 '24 11:05 dyc3

Todo list of remaining tasks, mostly for myself.

  • [x] Refactor as discussed here: https://github.com/biomejs/biome/pull/2884#discussion_r1605721658 (#2922)
  • [x] Address this: https://github.com/biomejs/biome/pull/2943#issuecomment-2124154834
  • [ ] Use serde_path_to_error to print better diagnostics - (encountered issues with this)
    • Problem: does not work with flatten: https://github.com/dtolnay/path-to-error/issues/9
    • This is unfortunately due to how serde works right now. ~~Might be possible to create a workaround.~~
    • Working around this would require effectively re implementing serde_path_to_error, which I don't think is a good use of time.
  • [x] Additional tests (#3219)
    • [x] add tests using check subcommand
  • [x] Add formatter config option use_editorconfig, default to false
  • [x] Support all editorconfig glob pattern syntax (eg. {a,b}, {num1..num2}) #3218
  • [x] set to enable by default when migrating from prettier

dyc3 avatar May 18 '24 13:05 dyc3

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.

ematipico avatar Jul 05 '24 05:07 ematipico

Well done, @dyc3 ! Thank you :)

kyr0 avatar Jul 06 '24 16:07 kyr0

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 avatar Jul 16 '24 02:07 junaga

oh and i think the docs need a fixup

junaga avatar Jul 16 '24 02:07 junaga

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.

dyc3 avatar Jul 16 '24 02:07 dyc3

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

nemchik avatar Jul 16 '24 03:07 nemchik

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 avatar Jul 16 '24 06:07 ematipico

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

ematipico avatar Jul 16 '24 06:07 ematipico

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.

nemchik avatar Jul 16 '24 13:07 nemchik

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.

ematipico avatar Jul 17 '24 05:07 ematipico

monorepo support without explicit opt-in?

junaga avatar Jul 17 '24 12:07 junaga

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

ematipico avatar Jul 17 '24 13:07 ematipico

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.

junaga avatar Jul 17 '24 14:07 junaga

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

ematipico avatar Jul 17 '24 15:07 ematipico

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

junaga avatar Jul 18 '24 02:07 junaga

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?

difosfor avatar Oct 11 '24 15:10 difosfor

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.

dyc3 avatar Oct 11 '24 16:10 dyc3

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.

difosfor avatar Oct 11 '24 16:10 difosfor