Reason: Consider lighter-weight formatting of parens

[jberdine]

Proposal: format parens in a more lisp style. E.g.: change

NodeSet.union
  acc
  (
    slice_nodes (
      IList.filter (fun s => not (NodeSet.mem s !visited)) n.nd_succs
    )
  )

to

NodeSet.union
  acc
  (slice_nodes
    (IList.filter (fun s => not (NodeSet.mem s !visited)) n.nd_succs))

and

switch (k1, k2) {
| (
  Prune_node is_true_branch1 if_kind1 descr1,
  Prune_node is_true_branch2 if_kind2 descr2
) => {

to

| (Prune_node is_true_branch1 if_kind1 descr1,
   Prune_node is_true_branch2 if_kind2 descr2) => {

and

IList.filter
  (
    fun
    | Sil.Aeq lhs rhs
    | Sil.Aneq lhs rhs => exp_contains lhs || exp_contains rhs
  )
  pi

to

IList.filter
  (fun
   | Sil.Aeq lhs rhs
   | Sil.Aneq lhs rhs => exp_contains lhs || exp_contains rhs)
  pi

and

(
  removed,
  if !Config.angelic_execution {
    remove_abducted_retvars p'
  } else {
    p'
  }
)

to

( removed,
  if !Config.angelic_execution {
    remove_abducted_retvars p'
  } else {
    p'
  } )

and

(
  not (Sys.file_exists dest_file_str) ||
    DB.file_modified_time (DB.filename_from_string source_file_str) >
      DB.file_modified_time dest_file
);

to (assuming #410)

(  not (Sys.file_exists dest_file_str)
|| DB.file_modified_time (DB.filename_from_string source_file_str) >
     DB.file_modified_time dest_file );

[jordwalke]

The switch statement example you gave should be wrapping differently today. (Although it doesn't change anything about this feature request).

switch (k1, k2) {
| (
    Prune_node x y z x y z is_true_branch1 if_kind1 descr1,
    Another,
    Here,
    Prune_node is_true_branch2 if_kind2 descr2
  ) => {
    x: 10,
    y: 30
  }
};

[jordwalke]

You've also previously mentioned a middle-ground which I also like:

Instead of:

let x =
  callMyFunctionWith
    firstArg
    (
      reallyLongTupleItem x y z x y z x y z,
      reallyLongTupleItem x y z x y z x y z
    );

let x =
  callMyFunctionWith
    firstArg
    ( reallyLongTupleItem x y z x y z x y z,
      reallyLongTupleItem x y z x y z x y z
    );

The reason I liked that suggestion, is that you can still use your eyes to quickly balance all delimiters by simply scanning vertically any time you see an indentation (which is also true today), but we only needed to sacrifice one line instead of two. Also, all indentation remains a multiple of 2 as it does today.

I really do like the beauty of lisp style parens though - but I wonder if more people would like this middle ground.

[jordwalke]

Also, that middle-of-the-road indentation convention is a convention that could work universally, even with records or for parens that are not tuples, but resolve precedence of function application.

let x =
  callMyFunctionWith
    firstArg
    { reallyLongFieldName: f x y z x y z x y z,
      reallyLongFieldName: x y z x y z x y z
    };

let x =
  callMyFunctionWith
    firstArg
    ( reallyLongFuncName
        argOne
        argTwo
    );

All of those examples also maintain indentation levels that are multiples of 2, provides total consistency for all braces/parens, and can be visually inspected for balancing by scanning vertical lines across indentation.

Here's another example of this convention:

( not (Sys.file_exists dest_file_str)
  || DB.file_modified_time (DB.filename_from_string source_file_str)
     > DB.file_modified_time dest_file
);

[yunxing]

@jordwalke I like the middle ground more, my eyes are still not recovered from counting parens in lisp.

My personal opinion is that instead of

let x =
  callMyFunctionWith
    firstArg
    ( reallyLongTupleItem x y z x y z x y z,
      reallyLongTupleItem x y z x y z x y z
    );

I actually like

let x =
  callMyFunctionWith
    firstArg (
      reallyLongTupleItem x y z x y z x y z,
      reallyLongTupleItem x y z x y z x y z
    );

more.

[jordwalke]

@yunxing We can support that (And I think we do support that in many cases). This github issue can be thought of as what to do when that is not feasible. What is the fallback?

[jberdine]

The middle ground is definitely better, I think, than the current formatting. It _really_ makes me want to put the commas on the left though... :-) As for:

let x =
  callMyFunctionWith
    firstArg (
      reallyLongTupleItem x y z x y z x y z,
      reallyLongTupleItem x y z x y z x y z
    );

I think it is important to make sure that several parenthesized arguments also look reasonable:

let x =
  callMyFunctionWith
    firstArg (
      reallyLongTupleItem x y z x y z x y z,
      reallyLongTupleItem x y z x y z x y z
    )
    thirdArg (
      reallyLongTupleItem x y z x y z x y z,
      reallyLongTupleItem x y z x y z x y z
    );

In such cases I think that the other versions are better, particularly:

let x =
  callMyFunctionWith
    firstArg
    ( reallyLongTupleItem x y z x y z x y z,
      reallyLongTupleItem x y z x y z x y z )
    thirdArg
    ( reallyLongTupleItem x y z x y z x y z,
      reallyLongTupleItem x y z x y z x y z );

I just find the extra lines for the closing paren in the middle ground case to be a disruption. But not a huge deal, and probably mostly a matter of opinion.

[jordwalke]

Let's start with the middle ground and then revisit after another round of feedback. Personally, I'd be happy to use the Lisp form as well (even going so far as to eventually use an entirely paren based syntax believe it or not).

[yunxing]

@jberdine @jordwalke either solution looks better than the current one.

[andreasabel]

Commata on the left seems more principled:

let x =
  callMyFunctionWith
    firstArg
    ( reallyLongTupleItem x y z x y z x y z
    , reallyLongTupleItem x y z x y z x y z )
    thirdArg
    ( reallyLongTupleItem x y z x y z x y z
    , reallyLongTupleItem x y z x y z x y z );

[nmn]

As someone who's repeatedly failed to get comfortable reading lisp code because of closing parens, I'm extremely against putting closing delimiters on the same line as the wrapped code.

Being able to vertically scan closing delimiters is extremely important for readability.

I prefer this formatting the best:

let x =
  callMyFunctionWith
    firstArg
    ( reallyLongTupleItem x y z x y z x y z
    , reallyLongTupleItem x y z x y z x y z
    )
    thirdArg
    ( reallyLongTupleItem x y z x y z x y z
    , reallyLongTupleItem x y z x y z x y z
    );

[bsansouci]

I prefer @jberdine's original proposal. Lisp-like is more compact and I'd argue not less readable. If your code is very indent, it _will_ be hard to read, no matter where the parens are. The closing parens won't help you much in figuring out what's happening. I'd argue the indentation would be a better cue as to what is an argument to what.

[nmn]

My problem with lisp has always been that I can easily see when I enter a scope, but when you're mixing single line constructs with multi-line constructs, it can often get really tricky to see when a scope actually ends.

In the code example in this thread I can read every style just fine. But in complex deeply nested code, It gets really hard to track closing parens when you see ))))))) at the end of some lines.

[bsansouci]

@nmn I can understand the general argument, but I can't picture an actual example where such a nested call would benefit from newlines.

let test a b c => {
  complexFunction
    (arg1AlsoIsAFunction 
      (a + b) 
      (b * c)
      (longThirdArg a b (blabla (blablalba2 a b c))))
    (arg2AlsoIsAFunction
      (magicalThingsHappenHere
        (heyoThisIsSoFreakingLong 
          (a + 1000) 
          (b - 10000)
          (c * 10000))))
};

VS

let test a b c => {
  complexFunction
    (arg1AlsoIsAFunction 
      (a + b) 
      (b * c)
      (longThirdArg a b (blabla (blablalba2 a b c)))
    )
    (arg2AlsoIsAFunction
      (magicalThingsHappenHere
        (heyoThisIsSoFreakingLong 
          (a + 1000) 
          (b - 10000)
          (c * 10000)
        )
      )
    )
};

If I squint hard I can picture some case with a lot of expressions-as-statements being very confusing, but I'd assume that curly braces and semi colons would help delimitate visually.

[nmn]

@bsansouci In the given example, I have much easier time reading the second option. The thing here is that when I see a closing paren, I instantly know what it's closing. If it's at the end of a line, I need to go left, if it's at the starting of a line I need to scan up. In the first option, it really doesn't work that way.

Small point to make here. The first example here is pretty readable too because I can just follow the indentation instead of the parens. This works because whenever there is a multi-line function call, the first argument is on it's own line. If we do decide to go for the more concise syntax please don't let this happen:

(functionName arg1
  arg2
  arg3)

Maybe I'm a minority, and if I am, I'll make it work for me. I still manually reformat lisp code every time I need to read it.
But if the divide is not so clear, I don't really see a downside with the second option.

[jordwalke]

@nmn I definitely agree with you on the readability part - but this is something that we can easily support without removing your ability to format your code as you like. We need a good system for specifying a project's formatting configuation and using that to specify how to apply wrapping/parens, and number of spacing. I was thinking we could either place it in a .merlin file or package.json. @chenglou and I were thinking about having only a single file called package.json per project that would have everything you need to express/install/build dependencies, and I could even see putting other package related info in it such as project formatting configuration. From that one file we could install or generate whatever we needed to to work well with opam/merlin/npm etc.