-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: Create Editorconfig File as Part of Cargo Project #2549
Conversation
You can put a link to the RFC file, 0000-cargo-editorconfig.md Traditionally this is called Rendered |
First of all, thank you for the contribution. However, as discussed in the pre-RFC thread, I don't think this is a good idea to do by default. Having a cargo option to enable this, or putting it in a cargo template, or similar mechanisms all seem fine. Among other things:
Separately from that, an issue with the proposed defaults:
|
First of all, thank you for your feedback :)
I tried vim, atom and notepad++, none of them were consistent here without editorconfig.
As said in the RFC, that would be great, yet that is far away, let's not ignore a realistic solution in favor of some hypothetical future.
See the incomplete list of no plugin required here https://editorconfig.org/:
Normalized by usage, using this survey editorconfig is supported by 98.4% of development environments and rustfmt by 37.8% for web developers and 98.7% vs 28.3% for desktop developers respectively. That is a very big difference! Avoiding a solution for 2/3 of the community only because there might be support for rustfmt at some point in the future seems like a poor decision.
Vcs git is required for building?
Indeed that's unfortunate, one option would be to add warnings in rustfmt about this. Parsing editorconfig files should be relatively simple, plus there are already crates for this in the wild.
This is about psychology, if you have someone likely to change the defaults, I speculate they will also not be the people that use rustfmt with default settings, and at least with editorconfig they documented their change, and when I open a pull request for their project. I can avoid some needless friction.
All editors with editorconfig, be it default or with plugin, show an icon that editorconfig is active for the file. If the style conflicts with their previous style, they have the option to change editorconfig to match their previous style or remove the file. Both are very easy and shouldn't take much time. As always for defaults it is about finding the most common use case and expressing them without cannibalizing corner cases.
That argument seems very inconsistent, the rust book mentions the opinionated rust style in the first chapter at 'Hello World':
Either we as a community express opinionated formatting, or we don't. Saying we do in the rust book which is and should be the entry point for most people, and then shying away from expressing this in a configuration file seems off.
Editorconfig essentially only applies to files that are being edited, which usually means source code. Still, you are right, indent size and style and trim trailing whitspace can cause issues in other files. For example in markdown trailing whitespace has an impact on formatting.
This way a uncontroversial subset is applied to everything, and the opinionated things only apply for rust files. Of course there are cases where the global rules could be problematic, however they seem very corner case, and as said above in those cases, the user can edit or remove this file. |
On Mon, Sep 24, 2018 at 05:39:04PM +0000, Lukas Bergdoll wrote:
> Adding such default configuration to any editors that don't already have it would address this issue for every project, without needing these files. At that point, the editorconfig files would be entirely redundant, and would have spread through the ecosystem.
As said in the RFC, that would be great, yet that is far away, let's not ignore a realistic solution in favor of some hypothetical future.
It's not hypothetical; getting editors to set appropriate defaults is
quite feasible.
Normalized by usage, using this [survey](https://insights.stackoverflow.com/survey/2017#technology-most-popular-developer-environments-by-occupation) editorconfig is supported by 98.4% of development environments and rustfmt by 37.8% for web developers and 98.7% vs 28.3% for desktop developers respectively. That is a **very** big difference!
I'm having trouble interpreting the results from that survey, given that
the percentages don't add up to 100%; I'd assume that's a "check all the
environments you use" poll? I don't think you can extrapolate or add
with such percentages.
Avoiding a solution for 2/3 of the community only because there might be support for rustfmt at some point in the future seems like a poor decision.
I'm not talking about "editors might support rustfmt", I'm talking about
that projects and people can use `rustfmt` or `cargo fmt` themselves.
> This adds an extra file in the repository, and the first one not required to build the project. (Hence the suggestion above to make this optional and disabled by default.)
Vcs git is required for building?
I'm not sure what you mean by this.
> Ironically, this might well make it more likely that people would diverge from the standard Rust style, since it has the defaults visible right there for editing.
This is about psychology, if you have someone likely to change the defaults, I speculate they will also not be the people that use rustfmt with default settings, and at least with editorconfig they documented their change, and when I open a pull request for their project. I can avoid some needless friction.
Not what I mean. Built-in defaults don't call attention to themselves; a
`.editorconfig` file draws attention.
> The user may already have a prevailing style configured as part of a higher-level project, and this would override that.
All editors with editorconfig, be it default or with plugin, show an icon that editorconfig is active for the file.
I can't speak for other editors, but vim does not, AFAICT.
If the style conflicts with their previous style, they have the option to change editorconfig to match their previous style or remove the file. Both are very easy and shouldn't take much time. As always for defaults it is about finding the most common use case and expressing them without cannibalizing corner cases.
I think you might be underestimating the knee-jerk reactions people have
over indentation style and opinionated tools. :)
Consider projects trying to incrementally replace C with Rust, for instance.
Consider the difference between "here's this tool called rustfmt" and
"here, let me automatically format your code for you without asking".
I genuinely appreciate the goal of trying to provide defaults that work
well for people. I feel that creating a new file in every new project is
too much.
This feels like something best addressed via cargo templates, so that
you (or an organization) can easily choose to have this happen for all
new projects you create.
> On balance, I don't want anyone's first exposure to Rust to be tainted by a negative impression of "oh, this language is opinionated about formatting". The high cost of that, versus the minor annoyance of some projects diverging from the standard Rust style, does not favor a change like this.
That argument seems very inconsistent, the [rust book](https://doc.rust-lang.org/stable/book/second-edition/ch01-02-hello-world.html#anatomy-of-a-rust-program) mentions the opinionated rust style in the first chapter at 'Hello World':
There's a difference between having a default style and emitting files
that configure a user's editor.
> This should not apply to `*`, only to `*.rs` and `*.toml`.
Editorconfig essentially only applies to files that are being edited, which usually means source code. Still, you are right, indent size and style and trim trailing whitspace can cause issues in other files.
Also an issue for non-Rust source files, for instance.
How about:
```
# https://editorconfig.org
root = true
[*]
charset = utf-8
Not something you can safely assume for all projects (sadly).
end_of_line = lf
Not the correct assumption for all platforms, and IIRC not consistent
with existing behavior.
trim_trailing_whitespace = true
This is a problem. Highlighting trailing whitespace so people avoid
puting it in is a good idea. Silently deleting it is a problem, because
that can generate diff noise in unrelated commits.
|
I think having this in some kind of template is the best solution. I fear that there are lots of things various people would like This would also be a little difficult for Rustfmt. If there is an editorconfig file and a rustfmt.toml, which setting should win? If Rustfmt is used via an editor and the user changes the configuration, should that override a setting in the rustfmt.toml (we do this via the RLS, and it is controversial to say the least. Adding another source of defaults, seems to complicate things even more). |
Having gained a better understanding of the situation, I agree that adding editorconfig by default would be too much. Regarding cargo templates, there seem to be several tickets for it, with the most recent I found: rust-lang/cargo#5151. Are there supposed to be some default templates?
It still feels unsatisfying, I believe we can do more here than we are currently doing. For me it's less about enforcing the same style everywhere, I personally don't care too much about the style, it's more about removing friction when contributing to other projects. It's situations like these unbit/uwsgi#1719, that waste time without adding anything valuable. Give me some way to easily use your style. Editorconfig is great for this. One idea to mitigate this problem, would be to add a crates.io lint that checks 2 things, is the style consistent across some metrics for This creates 3 possibilities:
Of course consistency is not black and white it would most likely have to be a heuristic, partial results could also be displayed as such, 'matches rustfmt 97%'. To make this information obvious to crates.io users, it could be encoded as a badge, as example:
This would be setting the bar higher for projects that are open source and expect to be used and contributed to, by the community, which is my understanding of what crates.io is for. This way both crate maintainers and crate users are more aware of the situation. If we use this consistency score to influence crates.io searches, a potential side effect could be, that crate maintainers that want their crate to be popular would care about having some form of consistent style. Of course this could go horribly wrong, by making the crates.io contribution entry barrier feel needlessly high. |
We discussed this in the @rust-lang/cargo meeting today. We were quite receptive to the motivation and intention of this proposal, but we'd like to keep (For the record, we also explicitly discussed that the default of @rfcbot fcp close |
Team member @joshtriplett has proposed to close this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
I'm no longer on the cargo team; how do I get off the rfcbot list? I've manually removed myself from the ticky boxes list. |
Agree, I do not think we should have Cargo options for each and every little project scaffold property. Even Cargo’s existing VCS option is questionable. We should have a system for templates, which can be as rich or bare-bones as one may imagine. Rust programmers can create a repo of popular templates and Cargo can help choosing one when generating a new package directory.
No, since it specifies the desired behavior explicitly and in-tree rather than relying on editors and other tools used by people working on some Cargo package.
That is a hypothetical, I do not think this is realistic unless evidence is given otherwise. Adding EditorConfig is not just to configure text editors, it can also be used as part of e.g. a VCS hook that checks the validity (e.g., text encoding) of files under VCS. This is the case at @Structure-Systems.
This seems like a tangential issue. Whether it presently requires a little vs. no effort to use EditorConfig depending on the text editor used is not significant IMO.
How is that bad? Seems very far fetched. There are countless needless scripts and tidbits in most Git repos on GitHub.
Only insofar the settings overlap. IMO, they only do for indentation. A discrepancy between
The EditorConfig file can simply specify text encoding an nothing regarding code style. You’re forgetting about a very important application of EditorConfig files in software development. It’s not just source code files that are being put in source code bases, also text files/documents and structured config files. And this is true for Rust-based packages as well (at @Structure-Systems). There is no standard, cross-platform and cross-tooling method to prescribe let alone enforce the text encoding standard in such code bases, but EditorConfig.
Only if the EditorConfig file is generated and then kept. That means there are two moments where this situation can be prevented. E.g. generating it can be made non-default. You and I agree on this, but your criticism is leveled at EditorConfig in general rather than just generating an EditorConfig file by default for new Cargo packages.
I think you’re making too much of the prescriptions for style in EditorConfig files. There are none for Rust, just indentation (optionally). |
This comment has been minimized.
This comment has been minimized.
[Moderation note: One comment hidden. General questions about how teams conduct their business should be directed to the team as a whole, and asked in some other forum. Comments on GitHub threads should be focused on specific issues or PRs. Also, if you have concerns, please try to state them directly and not in the form of leading questions or insinuations.] |
Sounds like we're in agreement on that, then.
It's relevant to the desirability of supporting editorconfig by default rather than as a non-default item supported in templates.
Not in the ones I've worked with. More importantly, though, those "scripts and tidbits" are normally put in place by the project developers. I'd like to avoid giving the impression that Cargo creates piles of files people don't know about. Something worth noting: Cargo's creation of a .git repository is unlikely to be someone's first introduction to git. Cargo's creation of a .editorconfig file would likely be many people's first introduction to editorconfig.
I don't think that's of value to create by default; there's already a widespread default there, namely UTF-8. Also,
That seems fine; by all means let's have template components that include it.
Indentation is precisely what I meant. |
Copied from rust-lang/cargo#3506:
@Centril @joshtriplett given that me, the author of this RFC no longer believes in it, does it make sense to keep it open, or should we close it? |
@Voultapher its going through our process for closing by the team, just waiting on one more cargo team member to check their boxes, then it will sit in final comment period until its closed, unless something surprising happens during FCP. As the PR opener you're free to close it immediately if you want, of course. |
This comment has been minimized.
This comment has been minimized.
Moderation note: @sanmai-NL If you have concerns about our moderation practices, then please email the moderation team. Our email address is [email protected]. |
Note that the main value of my comment is pointing out how we’re having a needless discussion full of subjective opinion and irrelevant considerations. I see that you continue it now. That further supports my ‘hidden’ post.
That may be so, but whether to include it by default is irrelevant if the feature is off the table in the first place.
Check out https://github.com/rust-lang/rust again then.
If you say so. I do not agree Cargo should be Git-centric at all and I do not expect programmers to expect a Git repo by default. But again, this discussion is irrelevant.
False, not on Microsoft Windows.
Making too much of just that is what I meant then. |
This comment has been minimized.
This comment has been minimized.
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period, with a disposition to close, as per the review above, is now complete. By the power vested in me by Rust, I hereby close this RFC. |
Rendered