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

Add flag to control space around high precedence infix #2638

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

giltho
Copy link
Contributor

@giltho giltho commented Dec 13, 2024

Hello,

With version 0.27, and specifically #2580, things like this following code are formatted a bit weird:

let formula_base x =
let open Formula.Infix in
(Expr.typeof x) #== (Lit (Type IntType))
#&& (x #<= (Expr.int 4))
#&& ((Expr.int 0) #< x)

As follows:
let formula_base x =
let open Formula.Infix in
(Expr.typeof x)#==(Lit (Type IntType))#&&(x#<=(Expr.int 4))#&&((Expr.int 0)#<x)

It happens that a lot of my code looks like this (in fact this code is mine 🤣 - #1371). 0.27 makes these expressions very hard to read while they are supposed to look like arithmetics (or a OCaml DSL to build an AST).

This PR proposes a flag to disable this new behaviour.

Now, note that I understand where this comes from, and I acknowledge that this is somewhat necessary: ocamlformat rewrites some_function some_arg (a #+ b) into some_function some_arg a #+ b, and this is also hard to read. Another option from this PR would be to have the following conf instead:

high-precedence-operators={compact|parens}

where compact is the 0.27 choice and parens would do something like

some_function some_arg (a #: b)

so that precedence is made clear without sacrificing readability, but I have to admit I haven't quite been able to implement it with my limited knowledge of the codebase. Happy to discuss this :)

@Julow
Copy link
Collaborator

Julow commented Dec 16, 2024

The syntax of your DSL is surprising. For example, these two expressions are the same:

  Expr.typeof x #== Lit (Type IntType)
  Expr.typeof (x #== Lit) (Type IntType)

Also, ocamlformat doesn't apply disambiguation parentheses or doesn't try to preserve existing parentheses, which makes the DSL code harder to read:

-  ((Expr.typeof x)#==(Lit (Type IntType))#&&(x#<=(Expr.int 4)))#&&((Expr.int 0)#<x)
+  (Expr.typeof x)#==(Lit (Type IntType))#&&(x#<=(Expr.int 4))#&&((Expr.int 0)#<x)

I don't think OCamlformat should render these similarly to infix operators as they clearly parse differently. An option would solve your issue without changing what ocamlformat does by default and the high-precedence-operators=parens idea seems even better, as the output will be less confusing to read.
But I wonder if the easiest solution wouldn't be to change your DSL to use a more ocaml-like syntax ?

@giltho
Copy link
Contributor Author

giltho commented Dec 16, 2024

I should probably. I have to admit the issue is mostly aesthetics.

The characters that are valid in OCaml for infix operators are % ∣ < ∣ $ ∣ & ∣ * ∣ + ∣ - ∣ / ∣ = ∣ > ∣ @ ∣ ^ ∣ | ∣ # ∣ % ∣ < ∣ : ∣ .. Out of those, the only only characters that don't already have a strong connotation (arithmetic/boolean/float/lists) are $ and #. And $ has terrible ligatures under Fira Code, e.g.
image

All in all this is not major but it turns out that # is the nicest looking prefix for "logical" operator, and that it doesn't work as a post-fix...

@Julow
Copy link
Collaborator

Julow commented Dec 16, 2024

As @v-gb suggested in #2580, I also think that &&<something else> would work better. OCamlformat would function better and also you'll get the same precedence as the similar OCaml operators. I mean that &&$ has the same precedence as && and has a different precedence than ==$. Also, with the symbol at the end, the connotation problem perhaps goes away ?

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

Successfully merging this pull request may close these issues.

2 participants