Skip to content

Commit

Permalink
On top of ocaml-ppx#2298
Browse files Browse the repository at this point in the history
  • Loading branch information
Julow committed Mar 15, 2023
1 parent 73dd430 commit ab6ecc0
Show file tree
Hide file tree
Showing 10 changed files with 103 additions and 95 deletions.
2 changes: 1 addition & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
### Bug fixes

- Consistent indentation of `fun (type a) ->` that follow `fun x ->` (#2294, @Julow)
- Avoid adding breaks inside `~label:(fun` and base the indentation on the label. (#2271, #2291, #2293, @Julow)
- Avoid adding breaks inside `~label:(fun` and base the indentation on the label. (#2271, #2291, #2293, #2298, @Julow)
- Fix non-stabilizing comments attached to private/virtual/mutable keywords (#2272, @gpetiot)
- Fix formatting of comments in "disable" chunks (#2279, @gpetiot)
- Fix indentation of trailing double-semicolons (#2295, @gpetiot)
Expand Down
154 changes: 74 additions & 80 deletions lib/Fmt_ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1329,8 +1329,46 @@ and fmt_indexop_access c ctx ~fmt_atrs ~has_attr ~parens x =
fmt_assign_arrow c $ fmt_expression c (sub_exp ~ctx e) ) )
$ fmt_atrs ) )

and fmt_label_arg ?(box = true) ?epi ?parens ?eol c
(lbl, ({ast= arg; _} as xarg)) =
(** Format [Pexp_fun] or [Pexp_newtype]. [wrap_intro] wraps up to after the
[->] and is responsible for breaking. *)
and fmt_fun ?force_closing_paren
?(wrap_intro = fun x -> hvbox 2 x $ fmt "@ ") ?(box = true) ~label
?(parens = false) c ({ast; _} as xast) =
(* Side effects of Cmts.fmt c.cmts before Sugar.fun_ is important. *)
let cmt_before =
let eol =
match label with Nolabel -> None | _ -> Some (fmt "@;<9999 2>")
in
Cmts.fmt_before ?eol c ast.pexp_loc
in
let xargs, xbody = Sugar.fun_ c.cmts xast in
let fmt_cstr, xbody = type_constr_and_body c xbody in
let body =
let box =
match xbody.ast.pexp_desc with
| Pexp_fun _ | Pexp_newtype _ | Pexp_function _ -> Some false
| _ -> None
in
fmt_expression c ?box xbody
and closing =
if parens then closing_paren c ?force:force_closing_paren ~offset:(-2)
else noop
in
hovbox_if box 2
( wrap_intro
(hvbox 2
( hvbox 2
( hvbox 0
( fmt_label label ":" $ cmt_before $ fmt_if parens "("
$ fmt "fun" )
$ fmt "@ "
$ fmt_attributes c ast.pexp_attributes ~suf:" "
$ fmt_fun_args c xargs $ fmt_opt fmt_cstr )
$ fmt "@ ->" ) )
$ body $ closing
$ Cmts.fmt_after c ast.pexp_loc )
and fmt_label_arg ?(box = true) ?epi ?eol c (lbl, ({ast= arg; _} as xarg)) =
match (lbl, arg.pexp_desc) with
| (Labelled l | Optional l), Pexp_ident {txt= Lident i; loc}
when String.equal l i && List.is_empty arg.pexp_attributes ->
Expand All @@ -1348,45 +1386,23 @@ and fmt_label_arg ?(box = true) ?epi ?parens ?eol c
| Optional _ -> str "?"
| Nolabel -> noop
in
lbl $ fmt_expression c ~box ?epi ?parens xarg
lbl $ fmt_expression c ~box ?epi xarg
| (Labelled _ | Optional _), _ when Cmts.has_after c.cmts xarg.ast.pexp_loc
->
let cmts_after = Cmts.fmt_after c xarg.ast.pexp_loc in
hvbox_if box 2
( hvbox_if box 0
(fmt_expression c
~pro:(fmt_label lbl ":@;<0 2>")
~box ?epi ?parens xarg )
~box ?epi xarg )
$ cmts_after )
| (Labelled _ | Optional _), (Pexp_fun _ | Pexp_newtype _) ->
(* Side effects of Cmts.fmt c.cmts before Sugar.fun_ is important. *)
let cmt_before = Cmts.fmt_before c arg.pexp_loc in
let xargs, xbody = Sugar.fun_ c.cmts xarg in
let fmt_cstr, xbody = type_constr_and_body c xbody in
let body =
let box =
match xbody.ast.pexp_desc with
| Pexp_fun _ | Pexp_newtype _ | Pexp_function _ -> Some false
| _ -> None
in
fmt "@ " $ fmt_expression c ?box xbody
in
hovbox_if box 2
( hvbox 2
( hvbox 2
( hvbox 2 (fmt_label lbl ":" $ cmt_before $ fmt "(fun")
$ fmt "@ "
$ fmt_attributes c arg.pexp_attributes ~suf:" "
$ fmt_fun_args c xargs $ fmt_opt fmt_cstr )
$ fmt "@ ->" )
$ body
$ closing_paren c ~offset:(-2)
$ Cmts.fmt_after c arg.pexp_loc )
fmt_fun ~box ~label:lbl ~parens:true c xarg
| _ ->
let label_sep : s =
if box || c.conf.fmt_opts.wrap_fun_args.v then ":@," else ":"
in
fmt_label lbl label_sep $ fmt_expression c ~box ?epi ?parens xarg
fmt_label lbl label_sep $ fmt_expression c ~box ?epi xarg
and expression_width c xe =
String.length
Expand Down Expand Up @@ -1843,60 +1859,38 @@ and fmt_expression c ?(box = true) ?pro ?epi ?eol ?parens ?(indent_wrap = 0)
if c.conf.fmt_opts.wrap_fun_args.v then Fn.id else hvbox 2
in
match List.rev e1N1 with
| (lbl, ({pexp_desc= Pexp_fun _; pexp_loc; _} as eN1)) :: rev_e1N
when List.for_all rev_e1N ~f:(fun (_, eI) ->
| (lbl, ({pexp_desc= Pexp_fun (_, _, _, eN1_body); _} as eN1))
:: rev_args_before
when List.for_all rev_args_before ~f:(fun (_, eI) ->
is_simple c.conf (fun _ -> 0) (sub_exp ~ctx eI) ) ->
let e1N = List.rev rev_e1N in
(* Make sure the comment is placed after the eventual label but not
into the inner box if no label is present. Side effects of
Cmts.fmt c.cmts before Sugar.fun_ is important. *)
let cmts_outer, cmts_inner =
let cmt = Cmts.fmt_before c pexp_loc in
match lbl with Nolabel -> (cmt, noop) | _ -> (noop, cmt)
in
let xargs, xbody = Sugar.fun_ c.cmts (sub_exp ~ctx eN1) in
let fmt_cstr, xbody = type_constr_and_body c xbody in
let box =
match xbody.ast.pexp_desc with
| Pexp_fun _ | Pexp_function _ -> Some false
| _ -> None
in
let force =
if Location.is_single_line pexp_loc c.conf.fmt_opts.margin.v then
Fit
else Break
(* Last argument is a [fun _ ->]. *)
let args_before = List.rev rev_args_before in
let xlast_arg = sub_exp ~ctx eN1 in
let args =
let break_body =
match eN1_body.pexp_desc with
| Pexp_function _ -> fmt "@ "
| _ -> (
(* Avoid the "double indentation" of the application and the
function matching when the [max-indent] option is set. *)
match c.conf.fmt_opts.max_indent.v with
| Some i when i <= 2 -> fmt "@ "
| _ -> fmt "@;<1 2>" )
in
let wrap_intro x =
wrap (fmt_args_grouped e0 args_before $ fmt "@ " $ x)
$ break_body
in
let force_closing_paren =
if Location.is_single_line pexp_loc c.conf.fmt_opts.margin.v
then Fit
else Break
in
hovbox 0
(fmt_fun c ~force_closing_paren ~wrap_intro ~label:lbl
~parens:true xlast_arg )
in
hvbox 0
(Params.parens_if parens c.conf
(hovbox 0
( hovbox 2
( wrap
( fmt_args_grouped e0 e1N $ fmt "@ " $ cmts_outer
$ hvbox 2
( hvbox 2
( hvbox 0
( fmt_label lbl ":" $ cmts_inner
$ fmt "(fun" )
$ fmt "@ "
$ fmt_attributes c eN1.pexp_attributes
~suf:" "
$ fmt_fun_args c xargs $ fmt_opt fmt_cstr
)
$ fmt "@ ->" ) )
$ fmt
( match xbody.ast.pexp_desc with
| Pexp_function _ -> "@ "
| _ -> (
(* Avoid the "double indentation" of the
application and the function matching when the
[max-indent] option is set. *)
match c.conf.fmt_opts.max_indent.v with
| Some i when i <= 2 -> "@ "
| _ -> "@;<1 2>" ) )
$ fmt_expression c ?box xbody
$ closing_paren c ~force ~offset:(-2)
$ Cmts.fmt_after c pexp_loc )
$ fmt_atrs ) ) )
hvbox 0 (Params.parens_if parens c.conf (args $ fmt_atrs))
| ( lbl
, ( { pexp_desc= Pexp_function [{pc_lhs; pc_guard= None; pc_rhs}]
; pexp_loc
Expand Down
5 changes: 3 additions & 2 deletions test/passing/tests/eliom_ext.eliom
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ let%client () =
Eliom_client.onload
(* NB The service underlying the server_function isn't available on the
client before loading the page. *)
(fun () ->
Lwt.async (fun () -> log "Hello from the client to the server!") )
(fun
()
-> Lwt.async (fun () -> log "Hello from the client to the server!") )

let%client () =
Eliom_client.onload
Expand Down
5 changes: 5 additions & 0 deletions test/passing/tests/indicate_multiline_delimiters-cosl.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,8 @@ let contrived =
f aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
)
l

let contrived =
List.map l ~f:(fun aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ->
f aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
)
4 changes: 4 additions & 0 deletions test/passing/tests/indicate_multiline_delimiters-space.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,7 @@ let contrived =
~f:(fun aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ->
f aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa )
l

let contrived =
List.map l ~f:(fun aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ->
f aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa )
4 changes: 4 additions & 0 deletions test/passing/tests/indicate_multiline_delimiters.ml
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,7 @@ let contrived =
~f:(fun aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ->
f aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)
l

let contrived =
List.map l ~f:(fun aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ->
f aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)
18 changes: 9 additions & 9 deletions test/passing/tests/issue289.ml
Original file line number Diff line number Diff line change
Expand Up @@ -33,31 +33,31 @@ let foo =
~doc:"Toy ID."
~args:[]
~typ:(non_null guid)
~resolve:(fun _ctx x -> x.id)
~resolve:(fun _ctx x -> x.id )
; field
"name"
~doc:"Toy name."
~args:[]
~typ:(non_null string)
~resolve:(fun _ctx x -> x.name)
~resolve:(fun _ctx x -> x.name )
; field
"description"
~doc:"Toy description."
~args:[]
~typ:string
~resolve:(fun _ctx x -> x.description |> Util.option_of_string)
~resolve:(fun _ctx x -> x.description |> Util.option_of_string )
; field
"type"
~doc:"Toy type. Possible values are: car, animal, train."
~args:[]
~typ:(non_null toy_type_enum)
~resolve:(fun _ctx x -> x.toy_type)
~resolve:(fun _ctx x -> x.toy_type )
; field
"createdAt"
~doc:"Date created."
~args:[]
~typ:(non_null Scalar.date_time)
~resolve:(fun _ctx x -> x.created_at) ]
~resolve:(fun _ctx x -> x.created_at ) ]

[@@@ocamlformat "wrap-fun-args=true"]

Expand All @@ -82,14 +82,14 @@ let foo =
| AAAAAAAAAAAAAAAAAAAa -> x.idddddddddddddddddddddddddd
| BBBBBBBBBBBBBBBB -> ccccccccccccccccccccccc )
; field "id" ~doc:"Toy ID." ~args:[] ~typ:(non_null guid)
~resolve:(fun _ctx x -> x.id)
~resolve:(fun _ctx x -> x.id )
; field "name" ~doc:"Toy name." ~args:[] ~typ:(non_null string)
~resolve:(fun _ctx x -> x.name)
~resolve:(fun _ctx x -> x.name )
; field "description" ~doc:"Toy description." ~args:[] ~typ:string
~resolve:(fun _ctx x -> x.description |> Util.option_of_string)
~resolve:(fun _ctx x -> x.description |> Util.option_of_string )
; field "type" ~doc:"Toy type. Possible values are: car, animal, train."
~args:[] ~typ:(non_null toy_type_enum) ~resolve:(fun _ctx x ->
x.toy_type )
; field "createdAt" ~doc:"Date created." ~args:[]
~typ:(non_null Scalar.date_time) ~resolve:(fun _ctx x -> x.created_at)
~typ:(non_null Scalar.date_time) ~resolve:(fun _ctx x -> x.created_at )
]
2 changes: 1 addition & 1 deletion test/passing/tests/labelled_args-414.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ let () =
let () =
very_long_function_name
~very_long_argument_label:(* foo *)
(fun
(fun
very_long_argument_name_one
very_long_argument_name_two
very_long_argument_name_three
Expand Down
2 changes: 1 addition & 1 deletion test/passing/tests/labelled_args.ml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ let () =
let () =
very_long_function_name
~very_long_argument_label:(* foo *)
(fun
(fun
very_long_argument_name_one
very_long_argument_name_two
very_long_argument_name_three
Expand Down
2 changes: 1 addition & 1 deletion test/passing/tests/wrap_comments_break.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ let _ =
fffffffffff
aaaaaaaaaaaaaaaaaaaaaaaaaaaa
bbbbbbbbbbbbbbbbbbbbbbbbbb
~f:(fun x -> return xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx)
~f:(fun x -> return xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx )
in
2

0 comments on commit ab6ecc0

Please sign in to comment.