Add .editorconfig #81260

vn971 · 2021-01-22T08:54:19Z

This adds a .editorconfig file to rust-lang/rust, matching Clippy's. It's not clear that this will benefit many people, but the cost is low and the rewards are potentially meaningful.

rust-highfive · 2021-01-22T08:54:22Z

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

vn971 · 2021-01-22T09:22:46Z

By the way, the pull request itself does NOT have the .editorconfig removed, when I see it through github UI.
I wonder if a git merge went wrong there or smth like that.

ojeda · 2021-01-22T11:12:44Z

Was there at any time a .editorconfig in the main repository? From a quick look, the commits that added the file are from subtrees/submodules (miri and clippy), i.e. that added it to their own repository.

vn971 · 2021-01-22T14:40:56Z

@ojeda seems so: 9a008e2 , c2baf5f , 5eb4ab2

ojeda · 2021-01-22T14:56:50Z

Those do not come from rust-lang/rust; if you browse the files at those points in time and see the README.md, you will see they are from the clippy root etc. (e.g. https://github.com/rust-lang/rust/tree/c2baf5f2e303bc40ac370293c82927578b0ac3e4).

vn971 · 2021-01-22T15:27:08Z

Oh, I think you're right! Those aren't even submodules, but, from the looks of it, git trees merged directly into the git trees of rust-the-language within the same repository.

With that in mind, I guess the answer would change. Rust-the-language repo didn't have .editorconfig in its root directory before.

Given this context, would it make sense to add it? The commit would have to be rewritten to say "add" instead of "restore", but that's doable.

Mark-Simulacrum · 2021-01-24T21:42:25Z

Hm, so I am inclined to say that adding this wouldn't be necessary, but I don't know much about editor configs. Most files in Rust projects use the same basic settings so I'd guess this is probably not too necessary for most people. That said adding it does seem relatively low cost.

I'm going to nominate for T-compiler, perhaps someone feels more strongly. I am inclined to just r+ though if there aren't really any concerns, though it would be good to update the PR title and description now that we know it's not just being re-added. Maybe some folks from @rust-lang/clippy can say why they added or what the maintenance etc was like over time (I assume low to nonexistent).

vn971 · 2021-01-24T21:58:22Z

Hey, thanks. Given that it wasn't really a removal in the first place, I don't feel strongly about it and I'd be okay if we don't proceed with merging as well.

ojeda

In case it is decided to add .editorconfig in the end :-)

Also, please reword the commit & the title of the PR.

.editorconfig

flip1995 · 2021-01-25T10:13:31Z

So there are exactly two commits for the .editorconfig file.

rust-lang/rust-clippy@c2baf5f (added -> rust-lang/rust-clippy#1107)
rust-lang/rust-clippy@9a008e2 (yaml config -> rust-lang/rust-clippy#5031)

I really don't think this is necessary with rust files, because almost everyone uses rust-analyzer with a .rustfmt.toml file anyway. I personally don't even have any vim plugin installed that applies the .editorconfig file. I think it is in the Clippy repo, because someone added it (in 2015) and it doesn't hurt to have it.

If this should be added to rust-lang/rust, please keep it consistent with the .editorconfig file in the Clippy repo.

Mark-Simulacrum · 2021-01-28T15:30:18Z

Discussed during T-compiler meeting today; we are inclined to merge but I will need to review this to make sure it's consistent with our rules. If you could update the PR title and description to reflect the state (i.e., that we are not restoring) that would be good.

flip1995 · 2021-01-28T16:28:48Z

Please don't forget to also update the .editorconfig file in src/tools/clippy so that they are identical.

vn971 · 2021-01-28T16:43:45Z

@Mark-Simulacrum thanks! I've changed the title in the PR, ammended the commit to fix its wording and made .editorconfig identical between clippy and the root directory.

vn971 · 2021-01-28T16:45:47Z

I'm not sure if it makes the best sense to keep an .editorconfig in clippy separately. We could remove the clippy one altogether, which will make the root .editorconfig to be the only one and specifying what needs to be done.
(According to the specification, this file is searched recursively in parent directories.)

flip1995 · 2021-01-29T07:59:00Z

I'm not sure if it makes the best sense to keep an .editorconfig in clippy separately.

Clippy is mainly developed in a separate repo, which also benefits from a .editorconfig. Not having it may result in changes in the Clippy repo having to be reformatted when syncing Clippy to rustc.

flip1995 · 2021-01-29T08:00:16Z

src/tools/clippy/.editorconfig

 trim_trailing_whitespace = false

 [*.yml]
 indent_size = 2
+


extra newline?

Yes, to make it equal to the root .editorconfig and to follow best practices:

https://stackoverflow.com/questions/5813311/no-newline-at-end-of-file

Note that it is a good style to always put the newline as a last character if it is allowed by the file format. Furthermore, for example, for C and C++ header files it is required by the language standard.

https://unix.stackexchange.com/questions/18743/whats-the-point-in-adding-a-new-line-to-the-end-of-a-file

That is not important at all though, if desired, I can remove it here and in the other file. Really doesn't worth much time as far as I'm concerned.

Yes, but line 21 already has a trailing newline, so this is an extra newline. Otherwise the GH UI would display it like this:

@flip1995 I'm again not sure how important this is for you, but the current document in the PR has exactly one newline at the end of file:

Or as shown in github: https://github.com/rust-lang/rust/blob/af89037f19303a6b97dac6fbab45552c44c8c97a/.editorconfig

I mean this shouldn't be a blocker for merging this. But these are 2 newlines at the end of the file. One after line 21 and then one in line 22. See for example any other file in the repo, e.g.:

rust/rustfmt.toml

Line 35 in 9fa9b58

]

This file also has a newline at the end of the file, but GitHub (and most editors) don't display an extra newline after the last line in the document.

In other words if you run cat .editorconfig the output will be

$ cat .editorconfig ... indent_size = 2 $

If you run cat rustfmt.toml you get the output:

$ cat rustfmt.toml ... ] $

If you wouldn't have a newline at the end of the file, you would get the output

$ cat .editorconfig ... indent_size = 2$

(I'm aware that this is next level nitpicking)

Hey, thanks, I never knew editors (mousepad, nano, kate) display data this way. I confirmed with hexdump -C, however. I've removed the extra newline as you asked.

Mark-Simulacrum · 2021-02-02T17:09:19Z

r=me with commits squashed. It also looks like your git config has a different email from your github account (fine if you want that, just wanted to mention in case it's unintentional).

Editorconfig is a lightweight specification that helps maintaining consistent coding/formatting style accross editors, especially those editors that are not explicitly aware of Rust and rustfmt. https://editorconfig.org/

vn971 · 2021-02-02T17:14:06Z

@Mark-Simulacrum squashed & pushed

Mark-Simulacrum · 2021-02-02T17:21:51Z

@bors r+ rollup

bors · 2021-02-02T17:21:53Z

📌 Commit ae3164e has been approved by Mark-Simulacrum

…Simulacrum Add .editorconfig This adds a .editorconfig file to rust-lang/rust, matching Clippy's. It's not clear that this will benefit many people, but the cost is low and the rewards are potentially meaningful.

Rollup of 14 pull requests Successful merges: - rust-lang#80593 (Upgrade Chalk) - rust-lang#81260 (Add .editorconfig) - rust-lang#81455 (Add AArch64 big-endian and ILP32 targets) - rust-lang#81517 (Remove remnants of the santizer runtime crates from bootstrap) - rust-lang#81530 (sys: use `process::abort()` instead of `arch::wasm32::unreachable()`) - rust-lang#81544 (Add better diagnostic for unbounded Abst. Const) - rust-lang#81588 (Add doc aliases for "delete") - rust-lang#81603 (rustbuild: Don't build compiler twice for error-index-generator.) - rust-lang#81634 (Add long explanation e0521) - rust-lang#81636 (Directly use `Option<&[T]>` instead of converting from `Option<&Vec<T>>` later on) - rust-lang#81647 (Fix bug with assert!() calling the wrong edition of panic!().) - rust-lang#81655 (Improve wording of suggestion about accessing field) - rust-lang#81665 (Fix out of date `Scalar` documentation) - rust-lang#81671 (Add more associated type tests) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…Simulacrum Add .editorconfig This adds a .editorconfig file to rust-lang/rust, matching Clippy's. It's not clear that this will benefit many people, but the cost is low and the rewards are potentially meaningful.

rust-highfive assigned Mark-Simulacrum Jan 22, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 22, 2021

ojeda reviewed Jan 25, 2021

View reviewed changes

.editorconfig Outdated Show resolved Hide resolved

.editorconfig Outdated Show resolved Hide resolved

.editorconfig Show resolved Hide resolved

Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Jan 28, 2021

vn971 changed the title ~~Restore .editorconfig~~ Add .editorconfig Jan 28, 2021

vn971 force-pushed the restore-editorconfig branch 2 times, most recently from 7d59b09 to e88cd5c Compare January 28, 2021 16:38

flip1995 reviewed Jan 29, 2021

View reviewed changes

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 2, 2021

Add .editorconfig

ae3164e

Editorconfig is a lightweight specification that helps maintaining consistent coding/formatting style accross editors, especially those editors that are not explicitly aware of Rust and rustfmt. https://editorconfig.org/

vn971 force-pushed the restore-editorconfig branch from 0b115ed to ae3164e Compare February 2, 2021 17:14

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 2, 2021

m-ou-se mentioned this pull request Feb 2, 2021

Rollup of 8 pull requests #81674

Closed

This was referenced Feb 2, 2021

Rollup of 14 pull requests #81677

Closed

Rollup of 14 pull requests #81678

Merged

bors merged commit d91ce83 into rust-lang:master Feb 3, 2021

rustbot added this to the 1.51.0 milestone Feb 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add .editorconfig #81260

Add .editorconfig #81260

vn971 commented Jan 22, 2021 •

edited by Mark-Simulacrum

Loading

rust-highfive commented Jan 22, 2021

vn971 commented Jan 22, 2021

ojeda commented Jan 22, 2021 •

edited

Loading

vn971 commented Jan 22, 2021

ojeda commented Jan 22, 2021

vn971 commented Jan 22, 2021 •

edited

Loading

Mark-Simulacrum commented Jan 24, 2021

vn971 commented Jan 24, 2021

ojeda left a comment

flip1995 commented Jan 25, 2021 •

edited

Loading

Mark-Simulacrum commented Jan 28, 2021

flip1995 commented Jan 28, 2021

vn971 commented Jan 28, 2021

vn971 commented Jan 28, 2021

flip1995 commented Jan 29, 2021

flip1995 Jan 29, 2021

vn971 Jan 29, 2021

flip1995 Jan 29, 2021

vn971 Jan 29, 2021

flip1995 Jan 30, 2021 •

edited

Loading

vn971 Jan 30, 2021

Mark-Simulacrum commented Feb 2, 2021

vn971 commented Feb 2, 2021

Mark-Simulacrum commented Feb 2, 2021

bors commented Feb 2, 2021

Add .editorconfig #81260

Add .editorconfig #81260

Conversation

vn971 commented Jan 22, 2021 • edited by Mark-Simulacrum Loading

rust-highfive commented Jan 22, 2021

vn971 commented Jan 22, 2021

ojeda commented Jan 22, 2021 • edited Loading

vn971 commented Jan 22, 2021

ojeda commented Jan 22, 2021

vn971 commented Jan 22, 2021 • edited Loading

Mark-Simulacrum commented Jan 24, 2021

vn971 commented Jan 24, 2021

ojeda left a comment

Choose a reason for hiding this comment

flip1995 commented Jan 25, 2021 • edited Loading

Mark-Simulacrum commented Jan 28, 2021

flip1995 commented Jan 28, 2021

vn971 commented Jan 28, 2021

vn971 commented Jan 28, 2021

flip1995 commented Jan 29, 2021

flip1995 Jan 29, 2021

Choose a reason for hiding this comment

vn971 Jan 29, 2021

Choose a reason for hiding this comment

flip1995 Jan 29, 2021

Choose a reason for hiding this comment

vn971 Jan 29, 2021

Choose a reason for hiding this comment

flip1995 Jan 30, 2021 • edited Loading

Choose a reason for hiding this comment

vn971 Jan 30, 2021

Choose a reason for hiding this comment

Mark-Simulacrum commented Feb 2, 2021

vn971 commented Feb 2, 2021

Mark-Simulacrum commented Feb 2, 2021

bors commented Feb 2, 2021

vn971 commented Jan 22, 2021 •

edited by Mark-Simulacrum

Loading

ojeda commented Jan 22, 2021 •

edited

Loading

vn971 commented Jan 22, 2021 •

edited

Loading

flip1995 commented Jan 25, 2021 •

edited

Loading

flip1995 Jan 30, 2021 •

edited

Loading