nickel icon indicating copy to clipboard operation
nickel copied to clipboard

Bikesheding: formatting of multi-line expressions enclosed in parentheses

Open yannham opened this issue 1 year ago • 1 comments

After the addition of a new constructor for custom contracts (and in fact a primop for function-based contracts in general), the stdlib has many instances of the following pattern:

some_function (fun label value =>
   ...multiple lines of code
)

Or sometimes with multiple arguments (some_function is most of the time std.contract.custom or the primop %contract/custom%). Nickel currently formats such multi-line applications in the following way:

  • put some_function on its own line
  • indent arguments (here, the function) and put it on the next line
  • because ( ... ) is multi-line as well, put ( on the first line, and add another indentation level
  • put the content of the function
  • put a closing )

This leads the std.contract.custom pattern to add a lot of indentation. But it's not the only offending example: any higher-order function like std.array.map or various folds also exhibit a big shift to the right of the screen, and such calls might be nested. Here is an example from the stdlib:

= fun contracts =>
  %contract/custom%
    (
      fun label value =>
        std.array.try_fold_left
          (
            fun _acc Contract =>
              let label =
                %label/with_message%
                  "any_of: a delayed check of the picked branch failed"
                  label
              in
              std.contract.apply_as_custom Contract label value
              # We want to short-circuit on contract success. Since try_fold_left
              # short-circuits on failure, we need to flip the two.
              |> match {
                'Ok value => 'Error value,
                'Error msg => 'Ok msg
              }
          )
          ('Ok null)
          contracts
        |> match {
          'Ok _ =>
            'Error {
              message = "any_of: value didn't match any of the contracts",
            },
          'Error value => 'Ok value,
        }
    ),

I think this formatting is sub-optimal: the content of the folded function is 6 indentation levels to the right of the original field definition. Another instance of high-indentation level, once again due to the way parentheses are formatted, is nested applications. Here is another example from the stdlib (here combined with anonymous function definitions)

DependentFun
  SliceIndexFirst
  (
    fun start_index =>
      DependentFun
        (SliceIndexSecond start_index)
        (
          fun end_index =>
            ArraySliceArray end_index -> Dyn
        )
  ),

Proposal(s)

I can see several ways to reduce the indentation level:

Special casing

  • as we do for field definitions, when the definitional = is followed by either a function, a record literal, an array, etc., apply a special casing that remove the indentation level and the new line after (
  • special case of an unary application where the argument is a function definition inside parentheses, that is cases like of std.contract.custom (fun label value ...) to not even add a newline between ( and fun, so that we could have
    std.contract.custom (fun label value =>
      'Ok value
    )
    

Both solutions aren't mutually exclusive.

Bulk un-indentation

A more brutal approach is to just not add a new line and an additional indentation level for stuff within parentheses. I think it's reasonable because containers (records and arrays) don't need parentheses anyway, so the most common use-cases for them is either to define anonymous functions or for nested applications. Those forms already add their own level of indentation. It's possible to find other cases, such as a long sequence of infix operators, but they don't seem be very common, and would probably fine as well. For example, here would be the result for a boolean expression:

(foo
&& bar
&& baz
&& qux
)

I did this bulk change on an experimental branch. You can see the result of formatting the stdlib. I think all changes are reasonable.

Closing parenthesis

A last minor point is, if we don't always add a newline after an opening ( in a multi-line context, should we keep ) on its own new line, as in:

std.contract.custom (fun label value =>
  'Ok value
)

Or rather put it on the last line of the content, as in:

std.contract.custom (fun label value =>
  'Ok value)

I'm personally in favor of the former, because I find it more readable to have matching delimiters on the same column. But one could argue that the latter is more symmetric (if we don't add a newline for (, we shouldn't add one for )).

Bikeshedding

To sum up, here are the points that I would like to open for discussion:

  • [ ] Should we special case ( depending on what follows, or should we always remove the currently added new line and the indentation level?

  • [ ] Should we put the last ) on its own line like before, or rather at the end of the previous expression?

  • [ ] Should we special case unary function application to a function definition to get something like

    foo_fun (fun bar baz =>
      ... some stuff ...
    )
    

    instead of

    foo_fun
      (fun bar => baz
        ... some stuff ...
      )
    

yannham avatar Jul 22 '24 11:07 yannham

I think ) should go on its own line. I don't have an opinion about the other choices; I think all the options presented are better than the current situation.

jneem avatar Aug 09 '24 08:08 jneem

A more brutal approach is to just not add a new line and an additional indentation level for stuff within parentheses. [...] I did this bulk change on an experimental branch. You can see the result of formatting the stdlib.

the "the result of formatting the stdlib" features the following format.

%contract/custom%
    (fun _label value =>
        if %typeof% value == 'Array then

from the previous description, i was expecting the following format.

%contract/custom%
    (fun _label value =>
    if %typeof% value == 'Array then

what am i missing?

prednaz avatar Sep 04 '24 16:09 prednaz

Should we special case ( depending on what follows, or should we always remove the currently added new line and the indentation level?

removing the new line makes sense whenever the next line is alone on its level of indentation, which is always the case for fun lines. for example, i would say it is a bad idea to remove it in a case like the following.

(
    let b = a in
    let c = b in
    c
)

Should we put the last ) on its own line like before, or rather at the end of the previous expression?

i slightly prefer it on its own line.

Should we special case unary function application to a function definition [...]

i vote against.

prednaz avatar Sep 04 '24 16:09 prednaz

what am i missing?

The parenthesis doesn't add indentation anymore, but a fun still does. When you write a multi-line function outside of any context, the body is indented with respect to the fun. Before, it would get the double indentation of both the parenthesis block AND the fun.

removing the new line makes sense whenever the next line is alone on its level of indentation, which is always the case for fun lines. for example, i would say it is a bad idea to remove it in a case like the following.

I agree, and that's what I ended up implementing. And actually we had precisely the same special cases for field definition, where a multiline definition foo = followed by either (, [, {, etc. will keep the opening delimiter dangling and then only add one level of indentation instead of two. So it's quite consistent with previous styling.

Should we special case unary function application to a function definition [...]

i vote against.

In the end I didn't, but I did special case all unary applications to use a space and not a newline to separate the function and the argument (and thus to not indent the argument). This has the following effect:

std.contract.custom (fun label value =>
  'Ok value
)

reverse [
 1,
 2,
 3,
]

check_condition (
  foo
  && bar
 || baz
)

apply_fun
  (fun value =>
    value + 1
  )
  5

std.array.map
  (fun x => x + 1)
  [1, 2, 3]

I think I like the result: for unary application, we only have one level of indentation when the argument is multiline (and it's not even added explicitly: naturally, an argument can only be an atom so is either very simple or enclosed by a construction that'll add the indentation, such as ( or {). However, for multi-ary application, it's clearer to put each argument on its own line, IMHO.

yannham avatar Sep 06 '24 11:09 yannham