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

Support .editorconfig #19455

Merged
merged 12 commits into from
Oct 21, 2024
Merged

Support .editorconfig #19455

merged 12 commits into from
Oct 21, 2024

Conversation

SomeoneToIgnore
Copy link
Contributor

Closes #8534
Supersedes #16349

Potential concerns:

  • we do not follow up to the / when looking for .editorconfig, only up to the worktree root.
    Seems fine for most of the cases, and the rest should be solved generically later, as the same issue exists for settings.json
  • fn language in AllLanguageSettings is very hot, called very frequently during rendering. We accumulate and parse all .editorconfig file contents beforehand, but have to go over globs and match these against the path given + merge the properties still.
    This does not seem to be very bad, but needs more testing and potentially some extra caching.

Release Notes:

  • Added .editorconfig support

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Oct 19, 2024
@SomeoneToIgnore SomeoneToIgnore merged commit d3cb08b into main Oct 21, 2024
11 checks passed
@SomeoneToIgnore SomeoneToIgnore deleted the kb/editorconfig branch October 21, 2024 10:05
@BuonOmo
Copy link
Contributor

BuonOmo commented Oct 22, 2024

Yeaaaaaaaaaah 🎉

Thank you 🌮

@cweagans
Copy link

This is one of the only feature requests that I've been watching closely, so I'm unclear on the release process. Now that this is merged, how does this make its way to the version of Zed running on my machine? I see that it's in 0.159.0-pre, but I'm not sure what the lead time is from that point to an actual stable release that gets pulled down through the auto-update functionality.

@notpeter
Copy link
Member

notpeter commented Oct 23, 2024

This is in Zed 0.159.x branch which is the current Zed Preview which released in the last hour. You can download it from here: https://zed.dev/releases/preview and try it out in Zed Preview today. (You can safely run Zed Preview side by side with Zed Stable).

Next Wednesday (2024-10-30) 0.159.x will become the next Zed Stable release and it will be available there.

More documentation here: https://zed.dev/docs/development/releases

@cweagans
Copy link

Nice! Thank you so much for the information. Really appreciate it!

@lode
Copy link

lode commented Oct 24, 2024

I just saw https://github.com/zed-industries/zed/releases/tag/v0.159.0-pre. I was expecting to see the editorconfig at the general improvements instead of at language. I'm not sure whether this is fully automated, but I think it would be good to move this to the general section for the real release. Maybe this is my bias :) but I think this deserves bigger visibility for people waiting for this.

@BuonOmo
Copy link
Contributor

BuonOmo commented Oct 24, 2024

👍 also because this is not a language improvement (e.g: we do not start supporting .editorconfig syntax or language) but a setting configuration, the languages section makes it harder to understand that.

@maxiim3
Copy link

maxiim3 commented Oct 31, 2024

Nice thank you !

@mibanez
Copy link

mibanez commented Nov 4, 2024

I'm working in a project that uses .editorconfig with max_line_length = 80. Some lines has more than 80 and now Zed is showing those lines soft-wrapped even when I'm explicitly setting "soft_wrap": "none". Is this the expected behavior?

@SomeoneToIgnore
Copy link
Contributor Author

.editorconfig always overrides whatever Zed settings, so, yes?

@mibanez
Copy link

mibanez commented Nov 4, 2024

That was what I thought, thanks, still it makes me noise why to override a setting that is not file format related but just a visual style preference.

@BuonOmo
Copy link
Contributor

BuonOmo commented Nov 4, 2024

@SomeoneToIgnore I think this actually might be a problem for multiple users: .editorconfig is here to enforce a writing standard but not a way to use your ide. Maybe we could consider not setting soft wrap but wrap guides instead ? That would hint a user to respect editorconfig without changing their habits? @mibanez wdyt?

Also I don't know if we're transmitting information from zed settings to language formatters, but if that is the case, then that is where the line length is interesting (no soft wrap, but hard wrap made by formatter directly!)

@SomeoneToIgnore
Copy link
Contributor Author

Seems simple to fix, if that's what's desired, please remove this block and check if it's working as expected for you:

let soft_wrap = if max_line_length.is_some() {
Some(SoftWrap::PreferredLineLength)
} else {
None
};

@mibanez
Copy link

mibanez commented Nov 4, 2024

Also I don't know if we're transmitting information from zed settings to language formatters, but if that is the case, then that is where the line length is interesting (no soft wrap, but hard wrap made by formatter directly!)

That would be awesome. We are using python with Ruff's formatter, its line-length rule is not a hard upper bound and a few exceptions are allowed some times. In any case, the wrap guide sounds nice to me.

please remove this block and check if it's working as expected for you

I'm trying this as soon as I can, ty!

@lode
Copy link

lode commented Nov 4, 2024

I would say this is not something that should be altered in Zed's implementation, but instead changed in those .editorconfig files. The specification (https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length) states max_line_length should force hard wrapping.

Though I also see lots of long discussions over at editorconfig (editorconfig/editorconfig#387) about this. But my take would be to follow the current specification (and the good use cases for this property, e.g. markdown) and use other (custom) rules if one wants other features.

@teohhanhui
Copy link

teohhanhui commented Nov 4, 2024

The problem then is that Zed's EditorConfig implementation incorrectly uses soft wrapping instead of the expected hard wrapping...

Or TL;DR of editorconfig/editorconfig#387 (comment) - EditorConfig is meant to enforce basic formatting standards in the file. Soft wrapping only affects the presentation, so it completely defeats the purpose of EditorConfig. Hard wrapping must be used in order to be able to effectively enforce the max line length.

@SomeoneToIgnore
Copy link
Contributor Author

Ok, no more wraps for us then: #20198
Zed does not have any "hard wrap" settings at all, ergo we'll ignore this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EditorConfig
8 participants