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

Add .editorconfig #1317

Closed
wants to merge 1 commit into from
Closed

Add .editorconfig #1317

wants to merge 1 commit into from

Conversation

silvanshade
Copy link
Contributor

This PR adds a basic EditorConfig file.

@silvanshade
Copy link
Contributor Author

It might be a good idea to add a separate section for [*.c] files and perhaps others if necessary.

[*]
charset = utf-8
end_of_line = lf
indent_size = 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular reason, just seems to be a popular choice. Some of the .c files seem to be using 2 spaces (example) and some of the .wat files also use 2 spaces (example). However, sometimes there is an inconsistent mix of 2 and 4 spaces (example).

I think it would be fine to leave indent_size unset if the choice is controversial.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think either way is fine. Our plan is to rewrite the remaining handful of .cpp files into Rust anyway :-).

@sunfishcode
Copy link
Member

One thing I'm unclear on about editorconfig files; we already have a .rustfmt file, and we use all default settings; shouldn't we expect editors to either read the .rustfmt file, or at least default to rustfmt's defaults?

@bjorn3
Copy link
Contributor

bjorn3 commented Mar 14, 2020

.editorconfig is meant to be supported by all editors, not just those that support rust. It also applies to all text files, not only rust files.

@silvanshade
Copy link
Contributor Author

@sunfishcode the .rustfmt is good but only applies to Rust files. Other things like .c or .wat or random text files won't get the benefit from that configuration.

The main thing I find useful about having the .editorconfig is that line endings are configured project-wide. I often use windows for development and prefer not to force the editor to LF globally since I sometimes edit system files as well. It's not critical or anything, just nice to have.

@abrown
Copy link
Contributor

abrown commented Mar 16, 2020

I think this might be extra-helpful if it told IDEs to wrap lines to a certain length; I tend to forget to do that for non-Rust files.

@sunfishcode
Copy link
Member

For LF line endings, should we add a .gitattributes file containing * text=auto eol=lf? That way git would handle it automatically for all text files, not just files created by editorconfig-aware editors.

@silvanshade
Copy link
Contributor Author

silvanshade commented Mar 18, 2020

Yeah the .gitattributes approach would also work for that. For reference, here is the rust-lang/rust/.gitattributes which does that.

@sunfishcode
Copy link
Member

I submitted #1370 to add a .gitattributes file, following your suggestion.

If anyone has a clear need for a .editorconfig file beyond that, we can add it, but otherwise my sense is to wait until we have one. #1365 has now removed most of the remaining C++ code from the repo, which further simplifies things.

@silvanshade
Copy link
Contributor Author

Sounds good. I'll close this in favor of the other PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants