Skip to content
This repository has been archived by the owner on Dec 21, 2023. It is now read-only.

Expression formatting #9

Open
yole opened this issue May 31, 2016 · 17 comments
Open

Expression formatting #9

yole opened this issue May 31, 2016 · 17 comments

Comments

@yole
Copy link
Contributor

yole commented May 31, 2016

When wrapping chained calls, put the . character or the ?. operator on the next line, with a single indent:

val anchor = owner.firstChild!!
    .siblings(forward = true)
    .dropWhile { it is PsiComment || it is PsiWhiteSpace }

Put the elvis operator on the line after the wrap:

val s = foo()
    ?: bar

Leave other operators before the wrap:

val s = "foo" + 
    "bar"
@yole yole closed this as completed May 31, 2016
@yole yole changed the title Ex;resskl Expression formatting May 31, 2016
@yole yole reopened this May 31, 2016
@damianw
Copy link

damianw commented Jun 8, 2016

It should be clarified whether the first call should always appear on the same line.

One edge case I've frequently run into: what should be done in the case where the first call (owner.firstChild!! in your example) has many arguments (relates to #8) or passes a long lambda?

// Option 1: Awkward
val anchor = owner.firstChild { child ->
  child is PsiElement
}.siblings(forward = true)
    .dropWhile { it is PsiComment || it is PsiWhiteSpace }

// Option 2: Awkward
val anchor = owner.firstChild { child ->
  child is PsiElement
}
    .siblings(forward = true)
    .dropWhile { it is PsiComment || it is PsiWhiteSpace }

// Option 3: Less awkward, IMO
val anchor = owner
    .firstChild { child ->
      child is PsiElement
    }
    .siblings(forward = true)
    .dropWhile { it is PsiComment || it is PsiWhiteSpace }

I've been preferring the third option.

@cypressious
Copy link

I prefer option 3

@damianw
Copy link

damianw commented Aug 2, 2016

One other thought related to #9 (comment):

If the first call is to a this-scoped or globally-scoped function, it should be placed on the next line:

val anchor =
    firstChild { child ->
      child is PsiElement
    }
    .siblings(forward = true)
    .dropWhile { it is PsiComment || it is PsiWhiteSpace }

@elizarov
Copy link

elizarov commented Nov 8, 2016

I'd prefer not to have any special rules with the respect to the first call in chain. It really depends on the context. It should be a requirement to always but "." on a new line, but if you do wrap the expression, then dot should be at the beginning of the line.

@elizarov
Copy link

elizarov commented Nov 8, 2016

It is not that clear about wrapping other operators. For example, when you write long algebraic expression it is very useful to wrap "+" and "-" so that they are on the beginning of the line. It is much easier to follow the formula in this case and it is consistent with math tradition.

@mykola-dev
Copy link

https://youtrack.jetbrains.com/issue/KT-9220
I prefer "align by dot":

    val anchor = owner.firstChild { child -> 
                          child is PsiElement
                      }
                      .siblings(forward = true)
                      .dropWhile { 
                          it is PsiComment || it is PsiWhiteSpace 
                      }

it plays nice in java

@Jeevuz
Copy link

Jeevuz commented Oct 5, 2018

Hey guys!
As I see from this discussion everybody preffer the option 3 from @damianw comment.
But IDEA still autoformats such case like in the second option:

val fooBars = mapOf(
    foo1 to bar1,
    foo2 to bar2
)
    .map { (foo, bar) ->
        FooBar(foo, bar)
     }

Is it ok or I just don't know how to make it format right?

@yole
Copy link
Contributor Author

yole commented Oct 8, 2018

@Jeevuz How do you expect this case to be formatted? Option 3 is about inserting a line break before the first . in the call chain. In this case there is no call chain.

The right way to format this example is as follows, and unless I'm mistaken IDEA does allow this formatting:

    val fooBars = mapOf(
        foo1 to bar1,
        foo2 to bar2
    ).map { (foo, bar) ->
        FooBar(foo, bar)
    }

@Jeevuz
Copy link

Jeevuz commented Oct 8, 2018

@yole Thanks for clarifying this! This exactly what I use now.

But I think this style hides parameters a little:

val secondExample = mapOf(
    longKey to firstValue,
    evenLongerKey to secondValue,
    keyThree to valueThreeWithSuffix
).map { (parameterName, value) -> // This line hides
    FooBar(parameterName, value)
}

Maybe it will be more readable with indented first call?

val secondExample = mapOf(
        longKey to firstValue,
        evenLongerKey to secondValue,
        keyThree to valueThreeWithSuffix
    )
    .map { (parameterName, value) ->
        FooBar(parameterName, value)
    }

Just curious, why the continuation indent (4 spaces) is not used for the method calls? I think it will be clearer that this method call is assigned to something.

@roschlau
Copy link

roschlau commented Oct 8, 2018

@yole @Jeevuz I think in that case I might even prefer this style, with the function call on a new line:

val fooBars =
    mapOf(
        foo1 to bar1,
        foo2 to bar2
    )
    .map { (foo, bar) ->
        FooBar(foo, bar)
    }

But I'd also probably put quite some effort into not having a function call that long in the first place if I need to chain it.

@Jeevuz
Copy link

Jeevuz commented Oct 9, 2018

Uh... I got the next level. AS formats as follows. How will you format this?

val fooBars = mapOf(
    foo1 to bar1,
    foo2 to bar2
).map { (foo, bar) ->
    FooBar(foo, bar)
}
    .filter { foo > 0 }

The only way I see is to define map creation as a separate local variable. But making a variable only for formatting purposes is little bit strange I think.

@roschlau
Copy link

roschlau commented Oct 10, 2018

@Jeevuz Like this?

val fooBars =
    mapOf(
        foo1 to bar1,
        foo2 to bar2
    )
    .map { (foo, bar) -> FooBar(foo, bar) }
    .filter { foo > 0 }

@Jeevuz
Copy link

Jeevuz commented Oct 10, 2018

@roschlau Exactly!
But AS doen't allow this type of formatting. For this to work we need first call params or lambda block to be idented when assigned a variable.
Maybe it's a topic for a new issue?

@guelo
Copy link

guelo commented Apr 27, 2020

Is there any justification for Leave other operators before the wrap:? I find it more readable to put boolean operators at the beginning of a long chain because it highlights how they are being combined

if (expression1
    && expr2
    && expr3
    || (expr4
        && expr5
        || expr6)
    && expr7
) {

}

@Vampire
Copy link

Vampire commented Jun 17, 2023

val s = "foo" + 
    "bar"

suggests when wrapping after a binary operator a single indent should be used.
ktlint also does it like this.
But IntelliJ uses a double-indent.
In which project should a bug be reported? :-)

@elizarov
Copy link

It goes here: https://youtrack.jetbrains.com/issues/KTIJ

@Vampire
Copy link

Vampire commented Jul 24, 2023

Thanks, https://youtrack.jetbrains.com/issue/KTIJ-26380

@elizarov elizarov reopened this Jul 24, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants