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

Poll: Should we support 2 spaces indentation? #43

Closed
shyiko opened this issue May 27, 2017 · 10 comments
Closed

Poll: Should we support 2 spaces indentation? #43

shyiko opened this issue May 27, 2017 · 10 comments

Comments

@shyiko
Copy link
Collaborator

@shyiko shyiko commented May 27, 2017

While official stance of JetBrains (citation needed) is 4 it's seems like we're alienating considerable chunk of the community (Square, Trello, etc.) by not allowing indentation with 2 spaces.

So the question is: should we allow 2 spaces for indentation or adhere to the official recommendation?

@shyiko shyiko added the poll label May 27, 2017
@shyiko
Copy link
Collaborator Author

@shyiko shyiko commented May 27, 2017

@PaulWoitaschek
Copy link

@PaulWoitaschek PaulWoitaschek commented May 27, 2017

Can't you add a config file where you specify stuff like that?

@shyiko
Copy link
Collaborator Author

@shyiko shyiko commented May 27, 2017

I could but that would be against the "no configuration" policy and we would be back in checkstyle territory. If there is a config file then why not to allow user to configure every aspect of ktlint? I mean, it requires minuscule amount of code and little effort, so why not?

The thing is, I want to keep ktlint simple for the user, keep it a "drop in" solution. You just add it and forget it (you can turn auto-correction (-F flag) on by default (as part of mvn verify / gradle check; or bind it to a commit hook) so that ktlint would do its job without bothering you too much).

Now, if there is a huge demand for 2 space indentation (and it seems like this is the case) - one way to tackle the problem is to have 2 ktlint artifacts (which have exactly the same set of rules with 1 distinction - indentation rule): ktlint (standard, 4 spaces indentation) and say ktlint-indent2 (2 spaces indentation adopted by Square). This is similar to standard vs semistandard.

shyiko added a commit to ktlint/ktlint.github.io that referenced this issue May 28, 2017
@shyiko shyiko changed the title Should we support 2 spaces indentation? Poll: Should we support 2 spaces indentation? May 28, 2017
@shyiko shyiko removed the poll label May 28, 2017
@shyiko
Copy link
Collaborator Author

@shyiko shyiko commented May 30, 2017

Another option: get the value from .editorconfig 🎉

@fwilhe
Copy link

@fwilhe fwilhe commented May 30, 2017

Read from .editorconfig if present and otherwise allow 2 or 4 spaces seems legit to me.

@PaulWoitaschek
Copy link

@PaulWoitaschek PaulWoitaschek commented May 30, 2017

I really like that idea with editor config. It's zero config drop in but respects the project settings.

@shyiko
Copy link
Collaborator Author

@shyiko shyiko commented May 30, 2017

@joshfriend @scottmeschke @tedyoung @fwilhe @PaulWoitaschek @mttmllns I'm happy to announce that this issue is effectively resolved. Starting from 0.8.0 ktlint checks .editorconfig for indent_size:

# within project's root directory
printf '[*.{kt,kts}]\nindent_size=2' >> .editorconfig
ktlint

🎈

P.S. Recommendation is still to use 4 spaces for indentation. Official position has not changed 😉

@shyiko shyiko closed this May 30, 2017
@PaulWoitaschek
Copy link

@PaulWoitaschek PaulWoitaschek commented May 30, 2017

Do you account the intendation intent too?

@shyiko
Copy link
Collaborator Author

@shyiko shyiko commented May 30, 2017

@PaulWoitaschek if you mean continuation_indent_size then no - not yet. Right now ktlint does a very basic check - "is indentation multiple of indent_size or not?".

@aem aem mentioned this issue Jul 18, 2017
@ZakTaccardi
Copy link

@ZakTaccardi ZakTaccardi commented Oct 17, 2017

you forgot about Google - they're 2 spaces too.

Tbh, 2 spaces is so much better than 4, but we all have AOSP to thank for that.

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

Successfully merging a pull request may close this issue.

None yet
4 participants