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

verticalMultiline disables optIn.configStyleArguments #1539

Closed
neko-kai opened this issue Oct 23, 2019 · 4 comments · Fixed by #1696
Closed

verticalMultiline disables optIn.configStyleArguments #1539

neko-kai opened this issue Oct 23, 2019 · 4 comments · Fixed by #1696
Labels

Comments

@neko-kai
Copy link

neko-kai commented Oct 23, 2019

  • Version: 2.2.1
  • Integration: IntelliJ
  • Configuration:
version = 2.2.1
maxColumn = 100

verticalMultiline.atDefnSite = true
optIn.configStyleArguments = true

Steps

Given code like this:

class Abc(
  x: Int, 
  b: Int
)

Problem

With verticalMultiline.atDefnSite = true scalafmt formats code like this:

class Abc(x: Int, b: Int)

i.e. the optIn.configStyleArguments=true option is ignored - the config list is not preserved on re-format.

Expectation

However, if verticalMultiline is disabled with verticalMultiline.atDefnSite = true, the output is:

class Abc(
  x: Int,
  b: Int
)

optIn.configStyleArguments=true works again – the config list is not squashed into one line despite being smaller than maxColumn

Workaround

None

@kitbellew
Copy link
Collaborator

@neko-kai vertical multiline, apparently, intends to fully control formatting regardless of source newlines.

given this, what would you expect to happen? what's the identical way it should format both

  class A(b, c)
  class D(
    e,
    f
  )

you can change its parameters to mimic config style, by reducing arity to 2, setting the flag to break after the opening parenthesis and, in this case, removing class from their exclusion list for f dangling closing parenthesis.

@neko-kai
Copy link
Author

@kitbellew

@neko-kai vertical multiline, apparently, intends to fully control formatting regardless of source newlines.
given this, what would you expect to happen? what's the identical way it should format both

Clearly this is unsolvable because I want these two to be non-identical, i.e. I'm turning on the optIn option to control what to break into multi-line and what to keep horizontal. For verticalMultiline that would mean that if the braces are already on separate lines (config-style) it can no longer reformat them back to horizontal and must assume minimal arity threshold for that section.

@kitbellew
Copy link
Collaborator

Clearly this is unsolvable because I want these two to be non-identical, i.e. I'm turning on the optIn option to control what to break into multi-line and what to keep horizontal. For verticalMultiline that would mean that if the braces are already on separate lines (config-style) it can no longer reformat them back to horizontal and must assume minimal arity threshold for that section.

if i may ask, why do you want to use vertical multiline if you'd like to control line breaks? is something missing in the non-vertical-multiline mode?

@neko-kai
Copy link
Author

neko-kai commented Feb 10, 2020

@kitbellew
Yes, multiple parameter lists in config-style force an empty line just for the braces:

def myMethod(
  arg1: Arg1,
  arg2: Arg2,
)(
  arg3: Arg3,
  arg4: Arg4,
)(
  arg5: Arg5,
  arg6: Arg6,
): Unit = ()

vs. verticalMultiline's compact formatting:

def myMethod(
    arg1: Arg1,
    arg2: Arg2,
  )(arg3: Arg3,
    arg4: Arg4,
  )(arg5: Arg5,
    arg6: Arg6,
  ): Unit = ()

kitbellew added a commit to kitbellew/scalafmt that referenced this issue Feb 10, 2020
kitbellew added a commit to kitbellew/scalafmt that referenced this issue Feb 10, 2020
If optin.configStyleArguments and/or newlines.danglingParentheses have
been explicitly specified, incorporate them into the vertical multiline
logic.

Fixes scalameta#1362.
Fixes scalameta#1539.
kitbellew added a commit to kitbellew/scalafmt that referenced this issue Mar 2, 2020
kitbellew added a commit to kitbellew/scalafmt that referenced this issue Mar 2, 2020
If optin.configStyleArguments is true and there's a break, preserve it.

Fixes scalameta#1539.
kitbellew added a commit that referenced this issue Mar 3, 2020
kitbellew added a commit that referenced this issue Mar 3, 2020
If optin.configStyleArguments is true and there's a break, preserve it.

Fixes #1539.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants