shwestrick / smlfmt

A custom parser/auto-formatter for Standard ML
MIT License
69 stars 15 forks source link

Grouping of function application #44

Open HarrisonGrodin opened 2 years ago

HarrisonGrodin commented 2 years ago

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 commented 2 years ago

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]
shwestrick commented 1 year ago

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.