Implement EditorConfig support

[TheDaemoness]

Closes #279.

This changeset currently implements partial support for EditorConfig. Here's what it includes:

• Full support for EditorConfig's globbing style.
• The indent_size, indent_style, tab_width, charset, and end_of_line properties.

Here's what it doesn't include:

• Support for non-standard charset values.
• insert_final_newline or trim_whitespace. Both of these should be possible to do with some changes to Document::save_impl, but a good implementation would allow these to be controlled from outside EditorConfig as well, and I think it would be better to do this in a separate PR.
• Some editors support max_line_length for doing hard wrapping. Helix does not.

[kirawi]

I'm not familiar with EditorConfig, but I think that it makes more sense to deserialize into a Rust struct (maybe one of the existing Config structs?) and then operate on that, to generalize to other forms of configuration.

[TheDaemoness]

I'm similarly unfamiliar with Helix's internals, so please correct me if any of my assumptions about them are wrong.

helix_term::Config appears to control editor-wide settings, such as the theme and what language servers should be used for what language.

That sort of configuration is out-of-scope for EditorConfig, which is more about declaring properties of individual files in a project, such as their encoding, line endings, indent style, maximum line length, formatting, and so on. The idea is that a project can put an .editorconfig file in its root, and contributors to that project can have their editors auto-configure themselves to comply with that project's style guidelines.

From what I could find, most of the variables that an EditorConfig file would call for changes to, in Helix, are in helix_view::Document. I wanted to do the parsing in Document::open and not Editor::open because the presence of an EditorConfig file potentially defeats the need for certain auto-detection that Helix does, and because parsing can be skipped if the file exists but cannot be opened.

Of course, if generalizing to other ways of setting these variables per-file is still important, that could still be done. I could define an object-safe trait and a couple of implementors for different forms of per-file editor configuration. It would probably keep the code of Document::open_with_config (or w/e) cleaner as well.

[kirawi]

I think that since we're already creating a Config type for Document, it might make more sense to encapsulate the already existing configuration fields for Document in Config. While this would make it so that Config is not EditorConfig-specific, I think it makes the code a lot cleaner.

What I would also do is make it so that, like is already done with language_loader, we could fill in Config from an EditorConfig if it exists (maybe from an Arc<EditorConfig>?). And then if using EditorConfig is undesirable, then it would just never fill in from an EditorConfig.

I'll think about this some more. I feel like there is a much better solution than what I proposed, using the Loader type to lazily load EditorConfigs.

[TheDaemoness]

I think that since we're already creating a Config type for Document, it might make more sense to encapsulate the already existing configuration fields for Document in Config.

That sounds good to me. One small quirk: Currently the fields of Config are Options, which they shouldn't be if a Config is going to be stored in a Document. However, if there's going to be some lazily-loaded EditorConfig type (or ConfigLoader or w/e), this isn't an issue at all. There might need to be some refactoring of the detect_*_impl methods to accommodate this.

I'll think about this some more.

Alright, I'll hold off on implementing any of these ideas for now.

[kirawi]

I think for now just lazily loading the configs based on the Document's language should be fine. If that's too complicated, then I'm fine without it too. My other suggestion to merge the config of the Document into one type might make things more complicated than they need to be.

Unless there's a reason it's not possible, I think it would be better to choose the config based on the language the Document is set to, rather than basing it on the path. This could be done by using the file-types field in languages.toml, with a fallback to using the path if no language has such a file-type.

[TheDaemoness]

Alright! I can finally prioritize this again! Apologies, I know it's been a while.

I think for now just lazily loading the configs based on the Document's language should be fine.

Unless there's a reason it's not possible, I think it would be better to choose the config based on the language the Document is set to, rather than basing it on the path

I think something got lost in communication. EditorConfig's purpose is to automate reconfiguring one's editor to match the style guidelines of a project, at least for certain common settings. It has no concept of language, and any concept of language it gains in the future would likely (speculation on my part) be in the form of a new key-value pair to specify that files matching a certain pattern are in a certain language. After all, language detection (and naming!) varies significantly from editor to editor. In short, what you're proposing is no longer EditorConfig. Sorry!

My other suggestion to merge the config of the Document into one type might make things more complicated than they need to be.

Alright, noted. I'll take a look at what's changed and try see how much of what I wrote can be adapted. I still need to finish ec4rs 1.0.0 before moving this out of draft status. I'll try and have something ready for review this weekend. If not this weekend, then the next.

[kirawi]

EditorConfig has globs for file extensions, and the Language config contains the extension as well. In retrospect, it would only make sense to look at the config if the buffer wasn't saved to a file yet. But I suppose it doesn't matter and goes against the config's model as it is only working on file extensions.

[TheDaemoness]

From the latest run:

error: package ec4rs v1.0.0 cannot be built because it requires rustc 1.59 or newer, while the currently active rustc version is 1.57.0

Is Rust 1.57 a hard requirement? ec4rs uses 1.59 because of destructuring assignments, but I should be able to put out a patch release that's compatible with an earlier version.

EDIT: It's been two weeks. I'm going to assume the answer is "yes". I'll try and push changes this weekend and mark this as ready for review so that this feature can get merged in some form.

[the-mikedavis]

Yep sry, we use 1.57 as an MSRV because it makes builds easier for distros like Void that might be a bit behind on their rust version: #1881

[TheDaemoness]

EditorConfig specifies properties for inserting a newline at the end of the file if there isn't one already,
as well as a trimming trailing whitespace after newlines. As far as I can tell, Helix doesn't allow one to control either of these things normally, meaning there's room to add these as a feature independent of EditorConfig. Should support for them be in a different PR?

If not, let me know and I can just implement them. Otherwise, marking this for review.

[the-mikedavis]

Yeah, implementing those can be separate 👍

We probably want to add commands for both of those things - they're definitely useful even if they aren't automatic/on-save

[dbarnett]

Fantastic to see an update here! 🎉

Are the code style concerns the same things commented inline, or could someone add comments if they're not? If @TheDaemoness isn't available for it I might be able to work on remaining touchups.

[kirawi]

As far as I'm aware, the other maintainers aside from Pascal have yet to comment on this PR so it definitely would be good to speak with them.

[the-mikedavis]

I agree with @pascalkuthe and I believe @archseer was thinking the same last time we discussed this

[TheDaemoness]

If I'm understanding the agreement correctly, that means the overall maintainer opinion is "it is a very invasive change pulling in a significant dependency and not an essential enough feature to warrant being implemented in core; would prefer to see this implemented as a plugin". Is there a rough estimate of when users can expect the plugin architecture to be in a state where it is capable of changing document options, including the encoding?

[kirawi]

I believe it is already able to, but cc @mattwparas

[dbarnett]

the overall maintainer opinion is "it is a very invasive change pulling in a significant dependency and not an essential enough feature to warrant being implemented in core ..."

FWIW it'd be very surprising to me to include language server support in core but not support for standard editor settings config. Or would language servers be moving into plugins as well...?

[kirawi]

From my understanding, Pascal preferred to add any options covered by editorconfig into Helix's config system if it didn't already exist.

[TheDaemoness]

From my understanding, Pascal preferred to add any options covered by editorconfig into Helix's config system if it didn't already exist.

This (emphasis mine) doesn't sound like expression of a preference to load .editorconfig files in Helix's config system to me.
I hope there's been a misunderstanding, because it sounds an awful lot like there is a preference to just have Helix's own way of configuring Helix, which defeats the entire point of EditorConfig (see blaggacao's comment).

It would be helpful if @pascalkuthe could clarify his preferences in this PR.

[the-mikedavis]

What Pascal is saying is that if there are missing config options that editorconfig exposes then we can add config options for those. For example insert_final_newline and #8157. Out of the box that would mean you could control those options with config.toml/languages.toml. Then an editorconfig plugin would be responsible for reading and parsing the editorconfig file and setting the equivalent options - the same way it works in Vim / Neovim for example.

[blaggacao]

Ok so, I understand a multiple PR approach:

1. Split out the part where new options are added and make them independently available to the helix config system. Rinse and repeat until all desired options are covered.
2. Prepare a version of this PR which can read the Editorconfig file and translate. Additional Bonus: make it a feature flag so that it can be used but doesn't commit the maintainer team for the long term in anticipation of a mandatory refactoring before graduating the feature once the config refactor landed.

There must be a way to not delay this feature any longer for affected users, while still preserving the legit long term interests of the maintainers. Or so I hope!

[the-mikedavis]

Point 1 sounds good regardless of editorconfig support. Though I think we have config options at least for what Vim / Neovim supports and what features we have. max_line_length is related to #2274 (not implemented), trim_trailing_whitespace is covered by #8366 and for spelling_language we don't have a spellchecker yet.

Point 2 is not what I had in mind though. We use feature flags for things like git that can be turned off to help compile to different targets like WASM: it's not meant to support features or dependencies we wouldn't otherwise support in core or to incubate / graduate features. If you want to use a feature that isn't supported in core (i.e. merged) you should run a branch from a PR or merge a PR's branch into a fork.

[TheDaemoness]

I assume the planned plugin system is https://github.com/mattwparas/helix/tree/steel-event-system (#8675) which appears to embed a novel Scheme interpreter to allow plugins to be written in Steel (a Scheme dialect). As far as I know there is no EditorConfig core in any dialect of Scheme, so it seems it would be necessary (or at least more practical) to make a Steel module out of an existing core.

I leave it to the maintainers to close this PR if it's decided this is the route they wish to go with. Best of luck to whoever opts to implement this plugin, and if I can be of assistance to that individual, I encourage them to reach out to me.

[jarkkojs]

Should definitely not require a plugin or anything. The most essential feature when contributing to upstream project.

I'm a Linux kernel maintainer so thus I did a workaround to languages.toml:

[[language]]
name = "c"
indent = { tab-width = 8, unit = "\t" }

For any other C based project I continue to use vim because otherwise I would have to constantly edit this file :-)

[jarkkojs]

First of all, I apologize for not bringing up the status of this PR with the rest of the team beforehand. I've spoken with @pascalkuthe on Matrix and he expressed reservations about merging this PR because he feels that it is a very invasive change pulling in a significant dependency and not an essential enough feature to warrant being implemented in core. He would prefer to see this implemented as a plugin in the future.

To be clear, this only reflects his personal opinion and he has not discussed this with the other maintainers, so this is not set in stone. Nonetheless, I understand that this is going to be very frustrating to hear given the amount of effort the author put into this PR and how much time has passed since this was first opened. I also understand that many of you who were waiting to see this PR merged will feel justifiably frustrated as well.

I cannot speak on the behalf of the maintainers, but I acknowledge that we could have done better in communicating with the community. However, I think it is also important to understand, and I know many of you already do, that it is difficult to achieve that relationship with the community with consideration to how the maintainers are volunteering their time to the project. Of course, this doesn't invalidate any frustration, but I hope it can provide some insight into the response.

Can live with a plugin and as a open source contributor I get the free time part.

Just want to remind that Rust is an exception on having only single coding convention. Most languages have multiple that are used depending on project you are working at... And editorconfig is the standard for fixing up that. For me personally it is more important feature than e.g. LSP.

[jarkkojs]

If I'm understanding the agreement correctly, that means the overall maintainer opinion is "it is a very invasive change pulling in a significant dependency and not an essential enough feature to warrant being implemented in core; would prefer to see this implemented as a plugin". Is there a rough estimate of when users can expect the plugin architecture to be in a state where it is capable of changing document options, including the encoding?

For me the core reason for ending up to try out Helix is that it does not have plugins :-) In vim (I use the regular vim) I have only few plugins mostly from tpope. I looked for something more modern and in terminal and Helix hit me because it has almost all I need but no plugins.

So definitely a bummer if this ends up to be a plugin (when there is even a plugin system in the first place) but having to install a single plugin is still sustainable :-) Would be better if not having to do that tho...

[Rudxain]

constantly edit this file

(this isn't a reply, just something I think it's worth mentioning, for anyone to read):

.helix/languages.toml-based workarounds:

• Add to local repo, but never to VCS. Fine for directories that have no VCS, but very error-prone if the VCS has a tendency to track everything that isn't in a .*ignore file (such as git).
• Add to local and .*ignore, but never stage .*ignore. Fine, even for VCS, but still prone to accidental committing.
• Add to local and .*ignore, but do stage and commit. This applies if the owners/maintainers of the repo are willing to include this change (if you are the owner, no problem). It's typical to find .vscode/ in .*ignore, so .helix/ is reasonable.
• Add to local, but ignore/exclude everything by default. This only applies if the owners/maintainers of the repo are willing to change how their VCS tracks files (if you are the owner, no problem)
• Commit config anyways! This only applies if the owners/maintainers of the repo are willing to include hx-specific files (if you are the owner, no problem). This is more "intrusive", but it's more convenient for other hx users that contribute to the same repo.

Each one of those approaches has its trade-offs, and it depends on the type of project.

Anyone, please let me know if there are more I haven't thought

[jarkkojs]

constantly edit this file

(this isn't a reply, just something I think it's worth mentioning, for anyone to read):

.helix/languages.toml-based workarounds:

* Add to local repo, but never to VCS. Fine for directories that have no VCS, but very error-prone if the VCS has a tendency to track everything that isn't in a `.*ignore` file (such as `git`).

* Add to local and `.*ignore`, but never stage `.*ignore`. Fine, even for VCS, but still prone to accidental committing.

* Add to local and `.*ignore`, but do stage and commit. This applies if the owners/maintainers of the repo are willing to include this change (if you are the owner, no problem). It's typical to find `.vscode/` in `.*ignore`, so `.helix/` is reasonable.

* Add to local, but `ignore`/`exclude` everything by default. This only applies if the owners/maintainers of the repo are willing to change how their VCS tracks files (if you are the owner, no problem)

* Commit config anyways! This only applies if the owners/maintainers of the repo are willing to include `hx`-specific files (if you are the owner, no problem). This is more "intrusive", but it's more convenient for other `hx` users that contribute to the same repo.

Each one of those approaches has its trade-offs, and it depends on the type of project.

Anyone, please let me know if there are more I haven't thought

It would be probably more effective to have one of these (not sure if XOR or OR):

1. HELIX_CONFIG environment variable pointing out to a non-default config directory rather than hardcode alternative config directory.
2. A command line parameter -c pointing out to the alternative config directory and optionally a HELIX_FLAGS where you could specify a string appended to the command-line containing extra parameters.

Please consider all my comments not requirements. As said I fully get the "free time" aspect. So e.g. here if this was done in the first place, these refinements could make it better...

[Rudxain]

2. A command line parameter -c

It already exists! Full name is --config, -c is the short alias

[bentenjamin]

I'd love to see this implemented as a core feature. nixpkgs is a massive project that uses them for example

[senekor]

Re: .helix/languages.toml-based workarounds. A .gitignore file can be added to the .helix directory with *, to ignore everything inside of it. This is a nice way to keep the helix-specific stuff out of VCS without having to convince maintainers to update their .gitignore.

(btw. the linux kernel already has a .* in its gitignore, so that's not even needed there.)

I don't have enough contact with editorconfig myself to put in the effort, but I had the idea of a CLI that "transpiles" .editorconfig to .helix/languages.toml (plus generates the mentioned .helix/.gitignore). This could live completely independent of helix, no plugin system needed.

Those who want a maximally smooth experience may write a shell hook that automatically calls this CLI if a .editorconfig file is detected when cd'ing into a directory. Clone a new repo, cd into it, open helix... and it's already correctly configured.

I'm not sure how complicated of a task it is, but I'm guessing the pareto principle applies.

[thecaralice]

Regarding .gitignore, there's a better option: global git ignore (~/.config/git/ignore IIRC); for a specific project, .git/info/exclude is also an option.

[senekor]

Regarding the CLI idea, one issue is that arbitrary globs for which files a config applies to doesn't nicely match helix' languages.toml format, because that assumes "one config per language". e.g. from the linux kernel:

[*.{json,py,rs}]
charset = utf-8
end_of_line = lf
insert_final_newline = true
indent_style = space
indent_size = 4

# this must be below the general *.py to overwrite it
[tools/{perf,power,rcu,testing/kunit}/**.py,]
indent_style = tab
indent_size = 8

I think this is probably extremely rare, having different formatting configs for the same language in a single project. IMO it would be fine to extract only "top-level" file extensions from the section headers and translate that into a helix languages.toml, ignoring complicated globs.

I guess it's technically possible to define a completely new language that only applies to the project-specific glob, but is otherwise identical to the real language. e.g.

[[language]]
name = "tools-python"
# ... duplicate entire config from python ...
file-types = [{ glob = "tools/{perf,power,rcu,testing/kunit}/**.py" }]
indent = { tab-width = 4, unit = "\t" }

but that seems hacky, brittle and generally not worth the effort.

[padhia]

Those who want a maximally smooth experience may write a shell hook that automatically calls this CLI if a .editorconfig file is detected when cd'ing into a directory. Clone a new repo, cd into it, open helix... and it's already correctly configured.

If a sufficiently large number of people end up doing this, wouldn't it be nice if Helix did this for them? After all, Helix is known as a batteries-included editor! .editorconfig is somewhat of a defacto tool to help enforce a consistent style for a project when many individual contributors might have different preferences. Almost all major editors that I know of support it as either a built-in capability or via a plugin.

[klardotsh]

Highly agree. Most editors with any noteworthy userbase bundle EC support into core. A few (perhaps most relevantly, Emacs, Micro, Sublime, and VSCode) require a plugin. None that I can tell require me to also set up shell hooks (or direnv, or whatever) to compile EC files into some editor-specific config format I ALSO need to remember to set up Git ignores for... and by the way, what happens if Helix ever splits into a core and GUIs, and now there's not a shell hook running on cd to compile these files down?

Just to keep it honest: that solution feels kludgey enough to me it'd be approximately like not having support at all. I consider myself a terminal power user but I have no interest in setting up stuff like that to fix a decision made by a text editor, it's less friction to just change my text editor at that point. Thankfully, most projects I work on have auto-formatting linters and so EC support isn't critical for most of them, but for the remaining projects... yowza.

[senekor]

I like "batteries-included" too, but I also understand the maintainers here. It is reasonable to leave editorconfig support up to a (future) plugin system. As mentioned, other popular editors have made that same choice, probably for the same reasons. Editorconfig is not that simple. (I find the linux kernel setup pretty deranged.)

I ALSO need to remember to set up Git ignores

No, the CLI can generate a .helix/.gitconfig with content *, that takes care of it.

If you don't like the shell-hook idea, don't add them. Run the CLI manually whenever you clone a repo, which doesn't happen that often.

that solution feels kludgey enough to me it'd be approximately like not having support at all [...] for the remaining projects... yowza

This exageration is contradictory. If not having editorconfig support is that terrible, running a single command once after cloning a repo would be worth it. I understand you want the 100% solution, but that's not an argument against the 90% solution.

[klardotsh]

My pushback was solely against the out-of-band CLI option, not the plugin system option.

[senekor]

I couldn't resist and cooked something up 🙂 Check it out if you're interested, I hope others find it useful. Issue reports welcome.

👉 ec2hx repo
👉 release v1.0.0 (including binaries, tested on linux)
👉 crates.io page

Installation using binstall works:

cargo binstall ec2hx

---

Call to Action

There is a tiny bit of information you can give me to make ec2hx potentially much better for you:

ec2hx cannot support arbitrary languages that are only covered by the global ([*]) editorconfig section and don't show up in the default languages.toml. These are probably niche or "underspecified" languages, e.g. *.txt. I can support them easily by hardcoding a list of their file globs. Please comment on this issue with file types you would like to be supported this way.

Edit: v1.1.0 added a --fallback-globs option you can use to provide a list of file globs for which to generate a language configuration with the global values. .txt is still included by default.

---

There are snapshot tests, so you can also post an issue with a .editorconfig file that's important to you and tell me in what way you'd like the output to be improved.

[senekor]

Update from ec2hx, my cursed idea to support arbitrary path globs is now implemented. It turned out less bad that I had expected, implementation wise.

The current v1.6.0 has lots of improvements, including support for trim_trailing_whitespace and max_line_length.