Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"if .. then .. else" is formatted inconsistently #495

Open
Marwes opened this issue Mar 21, 2018 · 9 comments
Open

"if .. then .. else" is formatted inconsistently #495

Marwes opened this issue Mar 21, 2018 · 9 comments

Comments

@Marwes
Copy link
Member

Marwes commented Mar 21, 2018

The auto formatting of https://github.com/gluon-lang/gluon#24 is inconsistent.

// Format 1
if x
then y
else z

// Format 2
if x then
    y
else
    z
@OvermindDL1
Copy link

I like the OCaml'y style of:

if blah
then bloop
else bleep

Unless used as a kind of early out or so then this common:

if not blah then bloop else
...more stuff...

@Marwes
Copy link
Member Author

Marwes commented Mar 22, 2018

@OvermindDL1 I am in two minds on this, Format 1 is nice and compact for the simple cases. On the other hand Format 2 works better when the true and false branches are more complicated.

// Format 1
if x
then
    let y = 1
    y
else
    let y = 1
    y

// Format 2
if x then
    let y = 1
    y
else
    let y = 1
    y

@OvermindDL1
Copy link

When the branches are complex then in OCaml I've always done:

if x then
    let y = 1 in
    y
else
    let y = 2 in
    y

So the Format 2 in your example.

/me really dislikes whitespace or newline sensitivity as a side note...

@Marwes
Copy link
Member Author

Marwes commented Mar 22, 2018

I have been bitten enough by indentation based syntax to prefer non-whitespace sensitivity but there are some unfortunate ambiguities that would need resolving to let that happen.

(* Fails since the last alternative is associated with the inner match *)
let x = match 1 with
    | 2 ->
        match () with
        | _ -> 1
    | _ -> 0
(* Syntax error *)
let x =
    let y = "a"
    y
print_string x;;

(* Add `in` to make it work *)
let x =
    let y = "a"
    in
    y
in
print_string x;;

Perhaps there is a a better syntax that would resolve these cases though 🤔

@OvermindDL1
Copy link

I have been bitten enough by indentation based syntax to prefer non-whitespace sensitivity but there are some unfortunate ambiguities that would need resolving to let that happen.

Well the OCaml usual way for those are:

let x = match 1 with
    | 2 ->
        begin match () with
        | _ -> 1
        end
    | _ -> 0

Also do not that the OCaml community-standard formatter/indenter ocp_indent would have made that case blaringly obvious as it would have moved the last case in your example to the right. :-)

As for the in, I really like the in bits as it makes it visually, syntactically, and greppably obvious where a statement ends, though I would have formatted it like:

let _ =
  let x =
    let y = "a" in
    y in
  print_string x

For note, ;; is never ever required in OCaml source code, it is only a command for the REPL to tell it to evaluate all code submitted to it so far, in source code it is a no-op. :-)

Also, I'd definitely recommend building a formatter into this system as well, being able to have the compiler output formatted source is a massive boon!

@Marwes
Copy link
Member Author

Marwes commented Mar 22, 2018

Also, I'd definitely recommend building a formatter into this system as well, being able to have the compiler output formatted source is a massive boon!

Like https://github.com/gluon-lang/gluon/tree/master/format ? 😎

Also do not that the OCaml community-standard formatter/indenter ocp_indent would have made that case blaringly obvious as it would have moved the last case in your example to the right. :-)

Tools or convenention can make it clear when one is doing it wrong. I don't like the idea of needing to help someone new with an issue and having the answer be "you should follow convention X or use tool Y".

As for the in, I really like the in bits as it makes it visually, syntactically, and greppably obvious where a statement ends, though I would have formatted it like:

It works ok in simple examples like this but since gluon has no concept of a top-level all the in tokens becomes really tedious. The main reason for the indentation based syntax was because of that actually, solving the match ambiguity was just gravy.

@OvermindDL1
Copy link

Like /format@master ? sunglasses

Whoo!

Tools or convenention can make it clear when one is doing it wrong. I don't like the idea of needing to help someone new with an issue and having the answer be "you should follow convention X or use tool Y".

Exactly why it should be baked into the compiler. The compiler could also help along by having a default enabled warning/error of when a match is used directly in the case of another match without an enclosing scope too.

It works ok in simple examples like this but since gluon has no concept of a top-level all the in tokens becomes really tedious.

I personally still wouldn't mind it but eh. :-)

The main reason for the indentation based syntax was because of that actually, solving the match ambiguity was just gravy.

Still exceptionally dislike whitespace sensitivity... ^.^;

@Marwes
Copy link
Member Author

Marwes commented Mar 22, 2018

For context this is the diff of the prelude after the syntax became whitespace sensitive dc78cf8#diff-0ff65e307e9232ed9edea0d444dd1f83

Exactly why it should be baked into the compiler. The compiler could also help along by having a default enabled warning/error of when a match is used directly in the case of another match without an enclosing scope too.

Perhaps. Needing to wrap parentheses around match is so ugly though :( May need to change the syntax a bit in that case. Rust works fine with {``} to enclose the alternatives of a match so perhaps gluon could do something like that as well.

@OvermindDL1
Copy link

I'd be good with {/} as well, OCaml uses begin/end instead as {/} has specific purposes, or of course parenthesis are an option but I find them ugly in such cases... ^.^;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants