Multiple parameters list

[daniel-trinh]

PR from mdr branch:
scala-ide#99

[bambuchaAdm]

10x for reactivating scalariform

[bambuchaAdm]

Do you think we should increase limit for parameter from 100 characters ?

[daniel-trinh]

For the limit, I think it's fine to have it as 1 to 100, since that's the same range of
AlignSingleLineCaseStatements.MaxArrowIndent at the moment.

This feature is a bit tricky to integrate when a parameter clause spans multiple lines:

Currently, enabling this feature would make something like this

def something(
  a: Int, 
  b: String, 
  c: Boolean)(
  d: Int, 
  e: String, 
  f: Boolean)

look like this:

def something(
  a: Int, 
  b: String, 
  c: Boolean)
             (
  d: Int, 
  e: String, 
  f: Boolean)

I think it would make sense to add a check so that all param clauses must be single line only
for this feature -- that way it won't look strange with multi line parameter clauses.

It would also make sense to implement this same behavior for method arguments as well:

  something(1, "", false)(2, "e", true)

would be rewritten to

  something(1, "", false)
           (2, "e", true)

[bambuchaAdm]

Should I add more integration test ? But what is right behavior of this feature when other are active ?

[bambuchaAdm]

I plan to create next PR but it can wait until we have consensus on how it should work.

[daniel-trinh]

@bambuchaAdm Are you still planning on working on this feature? If not, I'll pick it up as the next thing I implement.

More thoughts on implementation details:

I'm thinking that it might be simpler to not include extra configuration options in favor
of detecting whether or not the user wants their parameter clauses on multiple lines:

Examples:

This...

def a(one: Int)(two: String): Boolean

a(1)("2")

stays unchanged:

def a(one: Int)(two: String): Boolean

a(1)("2")

A newline between clauses...

def a(one: Int)
(two: String): Boolean

a(1)
("2")

becomes:

def a(one: Int)
     (two: String): Boolean

a(1)
 ("2")

[bambuchaAdm]

I'm going to finish this just after implement annotation. feasturue.

At second proposition. I have wrote minimal

object App extends Application {
  def mpl(i: Int)(s: String) = ???

  mpl(5)
     ("Ala")
}

and on compilation in SBT i have

[error] /tmp/test/App.scala:4: missing arguments for method mpl in object App;
[error] follow this method with `_' if you want to treat it as a partially applied function
[error]   mpl(5)
[error]      ^
[warn] /tmp/test/App.scala:5: a pure expression does nothing in statement position; you may be omitting necessary parentheses
[warn]      ("Ala")
[warn]       ^
[warn] one warning found

So we cannot use that syntax on callsite

[daniel-trinh]

OK, good to know. That simplifies the implementation a bit.

[bambuchaAdm]

I think is's work. But its need some refactoring. #7 will simplified implementation but i don't use it here.

[betehess]

What's the status of this PR?

[daniel-trinh]

@betehess I ran into conflicts trying to integrate this feature without breaking the alignParameter feature. This feature is on hold until those conflicts are fixed

[ktoso]

Any chances to revive this one, @bambuchaAdm? Bumped into needing it recently as well..

[bambuchaAdm]

Initial rework after changes on master. 1 test still fail. I think this should be ready to end of week.

[bambuchaAdm]

All tests are green. Ready to merge. @daniel-trinh please look on it quickly before conflicts appear.

[daniel-trinh]

It looks like this feature doesn't play well with the AlignParameters feature. Trying to merge these two functionalities and have them play nicely might be a hairy task. It might be best to simply disallow using the two features together. Thoughts?

Example of conflicts (seems like AlignParameter code overrides the BreakMultipleParameterGroups behavior):

// with preferences FormattingPreferences(Map(BreakMultipleParameterGroups -> true, BreakingThreshold -> 4, AlignParameters -> true)) in version 2.10.4 *** FAILED ***
   Format failure:
    ---- Expected ----
   def f(x: Int,¶
   y: Int)¶
        (y: Int, z: Int): Int = {¶
   }¶
   <<<
    ---- but was-----
   def f(¶
     x: Int,¶
     y: Int¶
   )(¶
     y: Int,¶
     z: Int¶
   ): Int = {¶
   }¶
   <<<
    ---- Original: -----
   def f(¶
     x: Int,¶
     y: Int¶
   )(¶
     y: Int,¶
     z: Int¶
   ): Int = {¶
   }¶

 // with preferences FormattingPreferences(Map(BreakMultipleParameterGroups -> true, BreakingThreshold -> 4, AlignParameters -> true)) in version 2.10.4 *** FAILED ***
   Format failure:
    ---- Expected ----
   def f(x: Int)¶
        (y: Int)¶
        (z: Int): Int = {¶
   }¶
   <<<
    ---- but was-----
   def f(x: Int)(y: Int)(z: Int): Int = {¶
   }¶
   <<<
    ---- Original: -----
   def f(x: Int)¶
        (y: Int)(z: Int): Int = {¶
   }¶

[jawshooah]

 ---- Expected ----
def f(x: Int,¶
y: Int)¶
     (y: Int, z: Int): Int = {¶
}¶
<<<

Is this really the expected result of AlignParameters and BreakMultipleParameterGroups interacting? Looks pretty ugly to my eyes.

[daniel-trinh]

It's not -- I actually haven't thought about the best way for the two to interact -- the failed examples are just to show that the behavior of enabling both is probably not going to result in something that the user expects.

[joprice]

+1

[kiritsuku]

Hey @bambuchaAdm, scalariform has new maintainers. Are you interested in bringing this PR back to live? You would have to rebase against the master branch of https://github.com/scala-ide/scalariform

[mjaghouri]

I am also interested in better formatting of curried parameter lists. Anything going to happen on that?

[kiritsuku]

See the issues on the upstream project https://github.com/scala-ide/scalariform, maybe there were changes in the meantime