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

Comments

Projects
None yet
4 participants
@shyiko
Copy link
Owner

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

This comment has been minimized.

Copy link
Owner

shyiko commented May 27, 2017

@PaulWoitaschek

This comment has been minimized.

Copy link

PaulWoitaschek commented May 27, 2017

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

@shyiko

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Owner

shyiko commented May 30, 2017

Another option: get the value from .editorconfig 🎉

@fwilhe

This comment has been minimized.

Copy link

fwilhe commented May 30, 2017

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

@PaulWoitaschek

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link

PaulWoitaschek commented May 30, 2017

Do you account the intendation intent too?

@shyiko

This comment has been minimized.

Copy link
Owner

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 referenced this issue Jul 18, 2017

Open

Wildcard imports #48

@ZakTaccardi

This comment has been minimized.

Copy link

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.

@JLLeitschuh JLLeitschuh referenced this issue Nov 9, 2017

Closed

Add Kotlin specific assertion helpers #1001

7 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment