tweag / topiary

https://topiary.tweag.io/
MIT License
558 stars 27 forks source link

Remove spacing around braces/brackets in Nickel? #407

Closed yannham closed 1 year ago

yannham commented 1 year ago

Is your feature request related to a problem? Please describe.

The Nickel formatter currently adds spaces after an opening { /[ and before the closing delimiter }/] on single line records and arrays (with an exception for single element lists):

{ foo = [ 1, 2, 3 ], bar = "bar" }

This looked unusual to me, as most language formatters I know don't do that and discussing with other members of the team, it seems to us than Nix is the only known precedent.

Describe the solution you'd like

The question part is thus: are you aware of other formatters adding spaces after the opening delimiter and before the closing delimiter of in-line records and arrays, beside Nix (I love Nix but it's clearly not a very good inspiration with respect to syntax/stylistic choices with its many idiosyncrasies :stuck_out_tongue: )? I wonder what Topiary do for OCaml as well. Nickel being an ML-like language with syntax clearly inspired from OCaml, it often makes sense to apply the same rationale to both languages.

If the answer is negative, then I would propose to do the common thing in Nickel as well, and ensure there are no spaces around delimiters.

Describe alternatives you've considered

Keep the current behavior.

cc @Xophmeister

Xophmeister commented 1 year ago

In the OCaml rules, there are no leading/trailing spaces for [...] and [|...|], but there are spaces for {...}:

$ topiary -locaml <<OCAML
> let _ = [1;2;3]
> let _ = [|1;2;3|]
> let _ = {foo=bar;baz=quux}
> OCAML

let _ = [1; 2; 3]
let _ = [|1; 2; 3|]
let _ = { foo = bar; baz = quux }

Our JSON rules (FWIW) do add the spacing in lists and objects:

$ topiary -ljson <<< '{"foo":[1,2,3]}'

{ "foo": [ 1, 2, 3 ] }

So... it's a bit inconsistent :sweat_smile:

yannham commented 1 year ago

Ah :sweat_smile: so let's base the decision on what other formatters do, so that we're less at risk of a bias loop?

Xophmeister commented 1 year ago

ocamlformat does this:

$ ocamlformat --enable-outside-detected-project --impl - <<OCAML
> let _ = [1;2;3]
> let _ = [|1;2;3|]
> let _ = {foo=bar;baz=quuz}
> OCAML

let _ = [ 1; 2; 3 ]
let _ = [| 1; 2; 3 |]
let _ = { foo = bar; baz = quuz }

It looks like it adds the spacing by default, but it's controllable through command line switches:

   --space-around-arrays
       Add a space inside the delimiters of arrays. The flag is set by
       default.

   --space-around-lists
       Add a space inside the delimiters of lists. The flag is set by
       default.

   --space-around-records
       Add a space inside the delimiters of records. The flag is set by
       default.

   --space-around-variants
       Add a space inside the delimiters of variants. The flag is set by
       default.

I found this Standard ML formatter. I didn't run it, but assuming they've dogfooded it, it looks like it's adding the spaces (e.g., see here).

Ormolu doesn't add the spaces for lists.

I can't find a JSON formatter that doesn't explode an object or list over multiple lines.

yannham commented 1 year ago

The team seems to prefer removing the spaces for array delimiters [/] and parentheses (/) but keep them around braces {/} and [|/|] delimiters.

Honestly at this stage it's not really driven by a strong rationale but mostly a matter of taste, given that formatters in the wild seem to do various things. I personally don't have any strong opinion :man_shrugging:

Xophmeister commented 1 year ago

Sounds good :+1: We'd obviously favour attested/community backed styles over consistency amongst different language rules, so I'll make this change...