Handle the Pexp_send _ case
#72
Conversation
Just like the `Pexp_field _` one. Cf. **@emillon**'s [comment][1] on ocaml-ppx#13. ```ocaml let f x = (x ()).contents let x obj = obj#hello () let x obj_f = (obj_f ())#hello () let f obj = obj#hello_some_pretty_long_one ~with_labels:true () let f obj = obj#hello_some_pretty_long_one ~with_labels:true "desjd\n\ dijsde\n" {md| In **markdown** |md} ``` [1]: ocaml-ppx#13 (comment)
src/Ast.ml
Outdated
| @@ -74,7 +74,7 @@ let would_force_break (c: Conf.t) s = | |||
| let rec is_trivial c exp = | |||
| match exp.pexp_desc with | |||
| | Pexp_constant Pconst_string (s, None) -> not (would_force_break c s) | |||
| | Pexp_constant _ | Pexp_field _ | Pexp_ident _ -> true | |||
| | Pexp_constant _ | Pexp_field _ | Pexp_send _ | Pexp_ident _ -> true | |||
There was a problem hiding this comment.
nit: was in alphabetic order before
src/Ast.ml
Outdated
| | Pexp_array _ | Pexp_field _ | Pexp_ident _ | Pexp_record _ | ||
| |Pexp_tuple _ | Pexp_variant _ | ||
| | Pexp_array _ | Pexp_field _ | Pexp_send _ | Pexp_ident _ | ||
| |Pexp_record _ | Pexp_tuple _ | Pexp_variant _ |
There was a problem hiding this comment.
nit: was in alphabetic order before
src/Ast.ml
Outdated
| @@ -647,6 +647,7 @@ end = struct | |||
| | Pexp_setfield (_, _, e0) when e0 == exp -> Some (LessMinus, Non) | |||
| | Pexp_setinstvar _ -> Some (LessMinus, Non) | |||
| | Pexp_field _ -> Some (Dot, Left) | |||
| | Pexp_send _ -> Some (HashOp, Left) | |||
There was a problem hiding this comment.
Isn't # non-associative?
There was a problem hiding this comment.
My understanding of this http://caml.inria.fr/pub/docs/manual-ocaml/expr.html is that it behaves like ..
Also it was already the decision there: https://github.com/smondet/ocamlformat/blob/pexp-send/src/Ast.ml#L171-L172 :)
Testing does not throw ast-diff-errors for all these:
let f x = (x ()).contents
let f x = (x ()).contents#method_of_ref ()
let f x = ((x ()).contents ())#method_of_ref ()
let f x = x#hello.contents.contents
let f x = x#hello.contents#bye.contents
let f x = x.contents#hello
let f x = x.contents#hello.contentsThere was a problem hiding this comment.
I'm not sure whether the manual is referring to the # send expression or the prefix ops that start with a # char, but the parser seems to treat them differently.
But you may be right that once code parses, then it might not be possible to trigger a formatting error. I don't know, but would rather match the parser if possible.
There was a problem hiding this comment.
With Non instead of Left for HashOp it gives something like this:
let f x = (x ()).contents
let f x = (x ()).contents#method_of_ref ()
let f x = ((x ()).contents ())#method_of_ref ()
let f x = (x#hello).contents.contents
let f x = ((x#hello).contents#bye).contents
let f x = x.contents#hello
let f x = (x.contents#hello).contents
My eyes/brain are trained to find parentheses helpful in those cases but I know it's not everyone's taste :)
Tell me if you prefer with Non and I'll commit that.
(PS the ops that start with # are not supposed to be part of the language itself, they are REPL-only, so in another chapter:
http://caml.inria.fr/pub/docs/manual-ocaml/toplevel.html
There is still another # for type definitions and for pattern mattching though ... OCaml syntax ❤️ )
There was a problem hiding this comment.
I think that the extra parens are due to the current order of Dot and HashOp being wrong in the definition of type prec. Fixing that and going with Non-associative seems like the right option.
(I think I saw somewhere that the # ops were intended/allowed for syntax extensions, not just the repl directives. Not sure. But yeah, the OCaml syntax is not exactly small...)
There was a problem hiding this comment.
(Ah right there is the js_of_ocaml ppx thing that converts ## and ##.!)
OK I'm changing the order in pred also 👍
|
Thanks! |
Just like the
Pexp_field _one.Cf. @emillon's comment on #13.