Skip to content

Create a .editorconfig file #59

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

Merged
merged 1 commit into from
Feb 23, 2018
Merged

Create a .editorconfig file #59

merged 1 commit into from
Feb 23, 2018

Conversation

Butt4cak3
Copy link
Contributor

@Butt4cak3 Butt4cak3 commented Feb 19, 2018

When merged, this pull request will add a configuration file for EditorConfig. EditorConfig is a collection of plugins that use the same configuration file. Plugins are available for many common text editors. Go to editorconfig.org for more information and plugin downloads.

The goal of adding such a configuration to the AAA is to make it easier for contributors to write consistently styled code throughout the entire project. The file will mainly contain two options for each language: "indent_style" and "indent_size" for spaces or "tab_width" for tabs.

This Pull Request exists as a place for discussion until we can agree on indentation styles for every language. Feel free to propose more languages that are not currently used in the AAA but could be in the future!

Here is a list of languages that will be included in the file.

@Butt4cak3
Copy link
Contributor Author

The reason "C and C++" is not yet checked in the list above is that not all of the C and C++ code in the repository uses the same indentation style. We should probably agree on one and modify the code so that it's consistent across the entire project.

@Gathros
Copy link
Contributor

Gathros commented Feb 19, 2018

I think C and C++ should be separated.

@leios
Copy link
Member

leios commented Feb 19, 2018

So, I think there has been a huge discussion on this in Issue #18 for C++

I am not sure about the other languages.

@Butt4cak3
Copy link
Contributor Author

@Gathros

I think C and C++ should be separated.

I would like another C or C++ programmer to get in on this and confirm that this change would make sense. I just thought that the two were syntactically so similar that it would be logical to use the same indentation method for both.

@strega-nil
Copy link

@Butt4cak3 it very much makes sense that C and C++ are different. They are very, very different languages, and are written very differently.

@strega-nil
Copy link

It's also good to point out that I haven't been using stroustrup style at all in the C++ code that I've written. I've moved back towards my own personal style. I can fix this, but if we actually want an editor config, it makes sense to use something similar to LLVM's style (because clang-format works very well on LLVM style).

@leios
Copy link
Member

leios commented Feb 20, 2018

To be fair, it is ultimately my job to maintain the style guidelines of the Algorithm Archive. I will be better about that in the future.

I think leaving the PR's out for a bit for code reviews is a good step in the right direction.

@strega-nil
Copy link

strega-nil commented Feb 20, 2018

I think that switching wholesale to LLVM style is the best way forward for C++ - this makes it incredibly easy to fix, and to format, and we don't really need to worry about anything - just run clang-format.

If we want to edit it slightly, I might do

AlignAfterOpenBracket: AlwaysBreak
AlwaysBreakTemplateDeclarations: true
BinPackParameters: false
BinPackArguments: false
PointerAlignment: Left

@leios
Copy link
Member

leios commented Feb 20, 2018

Here's a comparison between a few different styles: https://github.com/motine/cppstylelineup

@strega-nil
Copy link

https://gist.github.com/ubsan/32aa3b4e88f832fd436349bc33eb2b59 for examples of the styles.

@strega-nil
Copy link

strega-nil commented Feb 20, 2018

It doesn't look like .editorconfig is really supported by the rust and C++ communities, and I assume also for the Go community; it seems reasonable, for the languages with standard formatters, to use them instead of .editorconfig.

For Rust, I think we should probably use straight rustfmt.

For Go, I don't think there's really any options?

And for C++, I'd recommend the .clang-format I talked about in my comment above.

edit: this is also true of ocaml, with ocamlformat.

@Butt4cak3
Copy link
Contributor Author

Butt4cak3 commented Feb 20, 2018

@ubsan @leios @Gathros I'd like to ask you to move the discussion about general formatting to other places to leave room for other suggestions here. EditorConfig is only concerned with the type (tab/space) and width of indentation at the start of lines, not bracket placement and other whitespace (with the exception of trailing whitespace at the ends of lines).

I will split C and C++ up into two sections. I will leave them both at 4 spaces indentation until someone suggests something else.

for the languages with standard formatters, to use them instead of .editorconfig

I agree that we should use standard formatters for languages that have those, but EditorConfig serves a different purpose. Whether you let a program fix your formatting or not, your editor should still treat the tab key correctly to make it more convenient to view and edit the code.

Edit: In case of standard formatters with editor integration that could replace the functionality of EditorConfig, you should obviously use those instead.

@strega-nil
Copy link

Well then, the default for C++ should be two spaces. Four spaces in C++ is a rarity, ime, and major projects like LLVM, Chromium, and Firefox use 2-space.

@Butt4cak3
Copy link
Contributor Author

Alright. Thank you very much, @ubsan! C and C++ are now split up and I changed the C++ section to 2 space indentation.

By the way, I made an edit to my previous comment. Just making sure that you got that, because I think I made it shortly before you posted your last answer.

@zsparal
Copy link
Contributor

zsparal commented Feb 20, 2018

@ubsan @leios @Butt4cak3

  1. Go has gofmt which is pretty much the standard way to format Go source code. It uses tabs
  2. JavaScript/Markdown/web languages in general have prettier which has quite a bit of community support behind it. It uses 2 spaces
  3. Python should probably just use PEP8

@Butt4cak3 You commited with a user different from your GH one. Was it intentional?

@Butt4cak3
Copy link
Contributor Author

Butt4cak3 commented Feb 20, 2018

@Gustorn

Go has gofmt which is pretty much the standard way to format Go source code. It uses tabs

I'll add a section for *.go files with tab indention, then.

JavaScript/Markdown/web languages in general have prettier which has quite a bit of community support behind it. It uses 2 spaces

I rarely see JavaScript code with 2 space indentations. I don't think one can say that there is one single style for JavaScript that's commonly accepted. All the JS contribution in the AAA as of now use 4 spaces. I think I'll leave it like that.

When it comes to Markdown, I think going for 2 spaces is reasonable. I was just not sure, so I left it empty for the time being. I'll fill it in.

Python should probably just use PEP8

That's why I've set its indentation to 4 spaces.

You commited with a user different from your GH one. Was it intentional?

No, it was not. I made these changes on another computer and forgot to change my user information. It will be fine after the next rebase, I guess.

@strega-nil
Copy link

@Butt4cak3 Google, Mozilla, node.js, and npm are all examples of major JS projects/companies that use 2-space. I notice that webpack uses 4-space, but I definitely think that the majority of "major" JS projects are in 2-space.

@Butt4cak3
Copy link
Contributor Author

@ubsan You're right. Those major projects use 2 space indentation. I still have the impression that most people outside of those prefer 4 spaces. I can definitely see the case for 2 spaces, though.

I looked at a few style guides and their popularity and came to the conclusion that it's probably not a bad idea to adopt the 2 space indentation for this project. I'm sure @Gathros will have something to say about this as well.

@Gathros
Copy link
Contributor

Gathros commented Feb 21, 2018

@Butt4cak3 No I have nothing to say about that nor any opinion. I do prefer 4 spaces since things are clearer but I don't mind.

@zsparal
Copy link
Contributor

zsparal commented Feb 21, 2018

@Butt4cak3 I see you changed it already but a few more projects to support the 2 spaces argument for JS:

  1. Angular uses 2 spaces for indentation. All angular-cli generated projects also use 2 spaces by default (and people rarely change it)
  2. The same argument applies to React and Facebook in general
  3. Same for the other 2 major frameworks, ember and vue. All in all, pretty much every modern webapp is going to be using 2 spaces
  4. Some other major companies using 2 spaces: Microsoft, GitHub, Twitter

@Butt4cak3
Copy link
Contributor Author

What I could find for Clojure is that 2 space indentation is the most common and that's what the code submissions in the AAA use.

One of them uses a mix of tabs and spaces which is likely unintended. I don't know Clojure at all, so I won't touch the code, but maybe @earthfail can take a look at it since he submitted the code in the first place.

@earthfail
Copy link
Contributor

Well, @Butt4cak3 I actually moved my configs in that time so I
indented it by hand trying to align the inputs vertically. personally
I don't care about indentation but it is needed I will do it.

P.S: glad to be in a community 👍

@leios
Copy link
Member

leios commented Feb 21, 2018

Sorry if I missed this, but what was the final decision on C++?

2-space indentation or 4? I agree with @Gathros here, 4-spaces seems more readable, but I don't mind using 2 if it's the standard.

@strega-nil
Copy link

2-space indentation is the standard we've decided upon.

@Butt4cak3
Copy link
Contributor Author

Butt4cak3 commented Feb 22, 2018

@earthfail It seems like you used the right indentation scheme anyway, but one file contains mixed tabs and spaces. If you could clean that file up in the near future, that would be nice. Just get rid of the tabs and make sure it's properly indented.

@leios C++ is the one point on the list that's not done yet. @ubsan opened that other PR about C++ formatting. I already changed the .editorconfig file to 2 spaces, but I'm waiting for a decision to be made over there.

@leios
Copy link
Member

leios commented Feb 22, 2018

So, as far as the C++ conversation is concerned...

I don't mind any formatting, so long as it's consistent. because @ubsan and @Gathros submit the most C / C++ code, I think the discussion should be primarily between the. Ubsan wanted to go with 2 spaces, Gathros didn't mind... So I guess we are going with 2 spaces unless someone else wants to weigh in.

Also: ubsan seemed to indicate that it would be better to resolve this sooner than later in order to move on to other areas...

@jiegillet
Copy link
Member

jiegillet commented Feb 23, 2018

Can we add Elm to the list? Something like:

# Elm
[*.elm]
indent_style = space
indent_size = 2

@Butt4cak3
Copy link
Contributor Author

Butt4cak3 commented Feb 23, 2018

@leios Well, then. This PR doesn't hold anyone back. This config file is only for an optional text editor plugin.

@jiegillet Done! There's a link to the EditorConfig website in the top post. There is a plugin for Atom as well. It's convenient, but not required for contributions to the AAA.

@Butt4cak3
Copy link
Contributor Author

Alright. I added HTML and CSS and prepared the branch for a merge. (ping @leios)

@leios leios merged commit f9059d3 into algorithm-archivists:master Feb 23, 2018
@leios
Copy link
Member

leios commented Feb 23, 2018

WOO MERGED!

@Butt4cak3 Butt4cak3 deleted the editorconfig branch February 23, 2018 22:41
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.

7 participants