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

RFC: Create Editorconfig File as Part of Cargo Project #2549

Closed
wants to merge 1 commit into from

Conversation

Voultapher
Copy link

@Voultapher Voultapher commented Sep 23, 2018

@RustyYato
Copy link

RustyYato commented Sep 23, 2018

What do I put here?

You can put a link to the RFC file, 0000-cargo-editorconfig.md

Traditionally this is called Rendered

@Centril Centril added the T-cargo Relevant to the Cargo team, which will review and decide on the RFC. label Sep 23, 2018
@joshtriplett
Copy link
Member

joshtriplett commented Sep 24, 2018

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:

  • Many editors already have this as the default configuration for Rust; this configuration would be redundant in that case.
  • 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.
  • Of the editors mentioned that have editorconfig support, while "several" support it without plugins, many just have it as a plugin that requires installation and enabling, so counting that as "support" doesn't seem appropriate. What subset of editors would this "work out of the box" for?
  • 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.)
  • For projects that use rustfmt / cargo fmt, this would be redundant, and would add an extra place to change settings separate from rustfmt.toml.
  • 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.
  • The user may already have a prevailing style configured as part of a higher-level project, and this would override that.
  • While we can hope that most projects follow the prevailing Rust style, attempting to enforce that style via editor configuration seems likely to generate at least some surprise and pushback. Imagine, for instance, a large existing project with a standard style, who intend to use that same style for Rust code. 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.

Separately from that, an issue with the proposed defaults:

  • This should not apply to *, only to *.rs and *.toml.

@Voultapher
Copy link
Author

First of all, thank you for your feedback :)

Many editors already have this as the default configuration for Rust; this configuration would be redundant in that case.

I tried vim, atom and notepad++, none of them were consistent here without editorconfig.

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.

Of the editors mentioned that have editorconfig support, while "several" support it without plugins, many just have it as a plugin that requires installation and enabling, so counting that as "support" doesn't seem appropriate. What subset of editors would this "work out of the box" for?

See the incomplete list of no plugin required here https://editorconfig.org/:

  • GitHub
  • Gogs
  • IntelliJIdea
  • KtextEditor
  • Komodo
  • Kakoune
  • VisualStudio
  • PyCharm
  • ReSharper
  • Rider
  • RubyMine
  • SourceLair
  • TortoiseGit
  • WebStorm
  • Kate

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.

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?

For projects that use rustfmt / cargo fmt, this would be redundant, and would add an extra place to change settings separate from rustfmt.toml.

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.

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.

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. 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.

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 mentions the opinionated rust style in the first chapter at 'Hello World':

There are four important details to notice here. First, Rust style is to indent with four spaces, not a tab.

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.

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. For example in markdown trailing whitespace has an impact on formatting.
How about:

# https://editorconfig.org
root = true

[*]
charset = utf-8
end_of_line = lf

[*.{rs,toml}]
indent_size = 4
indent_style = space
insert_final_newline = true
trim_trailing_whitespace = true

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.

@joshtriplett
Copy link
Member

joshtriplett commented Sep 24, 2018 via email

@nrc
Copy link
Member

nrc commented Sep 24, 2018

Having a cargo option to enable this, or putting it in a cargo template, or similar mechanisms all seem fine.

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 cargo new to do, and adding them all, even as off-by-default options does not scale well.

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).

@Voultapher
Copy link
Author

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?

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.

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 *.rsand *.toml files, and is the style if consistent expressed somehow, ideally as rustfmt default or via customized rustfmt.toml.

This creates 3 possibilities:

  • The style is consistent and matches rustfmt defaults
  • The style is consistent and matches a custom style config, expressed in something like rustfmt.toml
  • The style is inconsistent

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:

  • No badge or [rustfmt consistent] green
  • [custom rustfmt consistent] neutral color or also green
  • [inconsistent formatting] red color

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.

@joshtriplett
Copy link
Member

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 cargo new only creating the minimal files needed for a new Rust project. Anything beyond that should be the domain of a project template.

(For the record, we also explicitly discussed that the default of --git doesn't necessarily fit that policy. Nonetheless, that's the existing behavior and widespread expectation.)

@rfcbot fcp close

@rfcbot
Copy link
Collaborator

rfcbot commented Sep 26, 2018

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.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-close This RFC is in PFCP or FCP with a disposition to close it. labels Sep 26, 2018
@carols10cents
Copy link
Member

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.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-close This RFC is in PFCP or FCP with a disposition to close it. labels Sep 26, 2018
@Centril
Copy link
Contributor

Centril commented Sep 26, 2018

@sanmai-NL
Copy link

sanmai-NL commented Oct 28, 2018

@joshtriplett:

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.

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.

Among other things:

* Many editors already have this as the default configuration for Rust; this configuration would be redundant in that case.

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.

* 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.

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.

* Of the editors mentioned that have editorconfig support, while "several" support it without plugins, many just have it as a plugin that requires installation and enabling, so counting that as "support" doesn't seem appropriate. What subset of editors would this "work out of the box" for?

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.

* 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.)

How is that bad? Seems very far fetched. There are countless needless scripts and tidbits in most Git repos on GitHub.

* For projects that use `rustfmt` / `cargo fmt`, this would be redundant, and would add an extra place to change settings separate from `rustfmt.toml`.

Only insofar the settings overlap. IMO, they only do for indentation. A discrepancy between rustfmt config and EditorConfig is not a serious problem. The maintainers and ultimately editors/IDEs decide how to handle precedence or how to simply avoid the slight overlap possible.

* 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.

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.

* The user may _already_ have a prevailing style configured as part of a higher-level project, and this would override that.

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.

* While we can hope that most projects follow the prevailing Rust style, attempting to _enforce_ that style via editor configuration seems likely to generate at least some surprise and pushback. Imagine, for instance, a large existing project with a standard style, who intend to use that same style for Rust code. 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.

I think you’re making too much of the prescriptions for style in EditorConfig files. There are none for Rust, just indentation (optionally).

@sanmai-NL

This comment has been minimized.

@mbrubeck
Copy link
Contributor

[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.]

@joshtriplett
Copy link
Member

@sanmai-NL

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.

Sounds like we're in agreement on that, then.

Whether it presently requires a little vs. no effort to use EditorConfig depending on the text editor used is not significant IMO.

It's relevant to the desirability of supporting editorconfig by default rather than as a non-default item supported in templates.

How is that bad? Seems very far fetched. There are countless needless scripts and tidbits in most Git repos on GitHub.

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.

The EditorConfig file can simply specify text encoding an nothing regarding code style.

I don't think that's of value to create by default; there's already a widespread default there, namely UTF-8. Also, .gitattributes files seem of more relevance there.

E.g. generating it can be made non-default.

That seems fine; by all means let's have template components that include it.

I think you’re making too much of the prescriptions for style in EditorConfig files. There are none for Rust, just indentation (optionally).

Indentation is precisely what I meant.

@Voultapher
Copy link
Author

Copied from rust-lang/cargo#3506:

Imo, the arguments made in #2549 also apply here, cargo should remain lightweight and only do build system and dependency management related things. Anything extra, like README, Editorconfig, CONTRIBUTING, etc. should be an ergonomic opt in with cargo templates. A popular one could be an OSS template, that has the 3 aforementioned files plus LICENSE.

If we start making exceptions for things like README, where do we draw the line? Also the cargo template story might be less clear, teaching that for anything that is not related to building the software like README files you can use cargo templates, is simple. Unfortunately there are already exceptions to this, like vcs, let's not make the situation worse.

@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?

@withoutboats
Copy link
Contributor

@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.

@sanmai-NL

This comment has been minimized.

@BurntSushi
Copy link
Member

Moderation note: @sanmai-NL If you have concerns about our moderation practices, then please email the moderation team. Our email address is rust-mods@rust-lang.org.

@sanmai-NL
Copy link

@sanmai-NL

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.

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.

Sounds like we're in agreement on that, then.

Whether it presently requires a little vs. no effort to use EditorConfig depending on the text editor used is not significant IMO.

It's relevant to the desirability of supporting editorconfig by default rather than as a non-default item supported in templates.

That may be so, but whether to include it by default is irrelevant if the feature is off the table in the first place.

How is that bad? Seems very far fetched. There are countless needless scripts and tidbits in most Git repos on GitHub.

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.

Check out https://github.com/rust-lang/rust again then. x.py, various CI configuration files, mailmap, etc.

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.

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.

The EditorConfig file can simply specify text encoding an nothing regarding code style.

I don't think that's of value to create by default; there's already a widespread default there, namely UTF-8. Also, .gitattributes files seem of more relevance there.

False, not on Microsoft Windows. .gitattributes is for Git, again, why assume people use Git?

E.g. generating it can be made non-default.

That seems fine; by all means let's have template components that include it.

I think you’re making too much of the prescriptions for style in EditorConfig files. There are none for Rust, just indentation (optionally).

Indentation is precisely what I meant.

Making too much of just that is what I meant then.

@sanmai-NL

This comment has been minimized.

@rfcbot
Copy link
Collaborator

rfcbot commented Nov 9, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Nov 9, 2018
@rfcbot rfcbot removed the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label Nov 9, 2018
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Nov 19, 2018
@rfcbot
Copy link
Collaborator

rfcbot commented Nov 19, 2018

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.

@rfcbot rfcbot added closed This FCP has been closed (as opposed to postponed) and removed disposition-close This RFC is in PFCP or FCP with a disposition to close it. labels Nov 19, 2018
@rfcbot rfcbot closed this Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed This FCP has been closed (as opposed to postponed) finished-final-comment-period The final comment period is finished for this RFC. T-cargo Relevant to the Cargo team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.