smlfmt icon indicating copy to clipboard operation
smlfmt copied to clipboard

Grouping of function application

Open HarrisonGrodin opened this issue 4 years ago • 2 comments

Currently,

val _ = this is an extremely long function application which must be wrapped onto additional lines due to its length

formats to

val _ =
  this is an extremely long function application which must be wrapped onto
    additional
    lines
    due
    to
    its
    length

It feels odd that the grouping happens per application; I would expect

val _ =
  this
    is
    an
    extremely
    long
    function
    application
    which
    must
    be
    wrapped
    onto
    additional
    lines
    due
    to
    its
    length

since the entire nested function application can't be grouped together.

My immediate thought for a solution would be to write a simple auxiliary function which traverses the left spine of an App until it reaches a non-App, performing the group only at the top level. It would be nice if we could accomplish this without violating compositionality, though... (That said, given how common curried function application is, perhaps it wouldn't be the worst place to add a special case?)

HarrisonGrodin avatar Dec 26 '21 21:12 HarrisonGrodin

For a more real-world example:

val _ =
  List.foldMapr (fn (_, SOME acc) => SOME acc | (x, NONE) => x)
    (fn x => if p x then SOME x else NONE)
    NONE
    [1, 2, 3, 4, 5, 6, 7, 8]

seems less preferable than

val _ =
  List.foldMapr
    (fn (_, SOME acc) => SOME acc | (x, NONE) => x)
    (fn x => if p x then SOME x else NONE)
    NONE
    [1, 2, 3, 4, 5, 6, 7, 8]

HarrisonGrodin avatar Dec 26 '21 21:12 HarrisonGrodin

Revisiting this now that smlfmt is more mature.

Here's our current output for these examples.

(* ========================================================================
 * max width: 80 *)
val _ =
  this is an extremely long function application which must be wrapped onto
    additional lines due to its length
val _ =
  List.foldMapr (fn (_, SOME acc) => SOME acc | (x, NONE) => x)
    (fn x => if p x then SOME x else NONE) NONE [1, 2, 3, 4, 5, 6, 7, 8]

(* ========================================================================
 * max width: 50 *)
val _ =
  this is an extremely long function application
    which must be wrapped onto additional lines
    due to its length

val _ =
  List.foldMapr
    (fn (_, SOME acc) => SOME acc | (x, NONE) => x)
    (fn x => if p x then SOME x else NONE) NONE
    [1, 2, 3, 4, 5, 6, 7, 8]

IMO this is not too bad, but could be improved. This seems closely related to #45.

shwestrick avatar Jan 23 '23 16:01 shwestrick