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

Why new OperatorWrapping rule? #163

Closed
Tolriq opened this issue Feb 28, 2018 · 8 comments
Closed

Why new OperatorWrapping rule? #163

Tolriq opened this issue Feb 28, 2018 · 8 comments

Comments

@Tolriq
Copy link

Tolriq commented Feb 28, 2018

Can someone point the source for that decision?

Currently default for Java with checkstyle and many is to break before those.
http://checkstyle.sourceforge.net/apidocs/com/puppycrawl/tools/checkstyle/checks/whitespace/OperatorWrapCheck.html

Kotlin guide https://android.github.io/kotlin-guides/style.html#where-to-break says:
"When a line is broken at a non-assignment operator the break comes before the symbol."

So it seems consensus was more to have the break before the operator and as such && || at the start of the new line. It allows way better reading.

If there's no way to return to previous way is there a way to manage that via editorconfig or something?

@SUPERCILEX
Copy link

This technically comes from the Kotlin style guide, but I disagree with them, that if statement looks gross IMO. There should be a way to go back to the old behavior.

@shyiko
Copy link
Collaborator

shyiko commented Feb 28, 2018

"When a line is broken at a non-assignment operator the break comes before the symbol." can lead (in case of +/-) to confusing situations like:

var v = 1
    - 2
// v == 1 not -1

(more on this at android/kotlin-guides#51).

&& / || at the EOL appears to be the default both in Kotlin Style Guide as @SUPERCILEX pointed out (+ in Kotlin/kotlin-style-guide#9 (comment)) and Google own code bases, e.g. https://github.com/android/android-studio-poet.

If decision is made (in android/kotlin-guides#51) to wrap before && / || - ktlint will be updated to follow that rule. Until then, Kotlin Style Guide version is enforced for consistency reasons (personally I don't care which one is selected as long as there is only one).

@shyiko shyiko closed this as completed Feb 28, 2018
@SUPERCILEX
Copy link

Alrighty, sounds fair.

@Tolriq
Copy link
Author

Tolriq commented Mar 1, 2018

@shyiko well thing is that for me consistency with the rest of the code base is more important :(
My apps are slowly migrating to Kotlin, consistency helps a lot to avoid human mistakes like missing parenthesis or operator orders so is mandatory at least during that phase even if keeping multiple years of habit would be nice.
If there's no way to have a choice on that I guess I'll be stuck at 0.15.1 :(

@kolboch
Copy link

kolboch commented Feb 14, 2020

Can this rule be disabled?

@Tapchicoma
Copy link
Collaborator

@kolboch yes, using disabled-rules: https://github.com/pinterest/ktlint#custom-editorconfig-properties

@kolboch
Copy link

kolboch commented Feb 15, 2020

@Tapchicoma , Thanks, actually found that later but thought it's still should be helpful as this discussion is something I found when investigating my issue. Right now I think I will make the same rule but with an exception to && and || operators. Do you think it is worth of pull request or that it is rather project-specific and should stay there to not mess up with guidelines?

@Tapchicoma
Copy link
Collaborator

Official styleguide is the target for this project rules, so in your case it should be project-specific custom rule.

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

No branches or pull requests

5 participants