Skip to content

Commit

Permalink
Remove incorrect fmt_expression ~epi mechanism (#2445)
Browse files Browse the repository at this point in the history
This argument doesn't work similarly to the `~epi` argument of other
functions. It meant different things depending on the expression:

- `Pexp_match` and `Pexp_apply`: it was similar to `~pro`.
- `Pexp_constant`: like the usual `~epi` but was passed a break.
- all other cases: it was not used.

These rules are now handled in `fmt_args_grouped`, the only place where
they made sense. The `~epi` argument is removed.

In the first case, the `~pro` argument is used instead. This requires
refactoring `fmt_expression` to make sure that every cases use `~pro`
exactly once.
  • Loading branch information
Julow authored Oct 5, 2023
1 parent ab3b75b commit d664fa2
Show file tree
Hide file tree
Showing 8 changed files with 616 additions and 500 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ profile. This started with version 0.26.0.
- Remove trailing space inside a wrapping empty signature (#2443, @Julow)
- Fix extension-point spacing in structures (#2450, @Julow)
- \* Consistent break after string constant argument (#2453, @Julow)
- Fix invalid syntax generated with `ocp-indent-compat` (#2445, @Julow)

## 0.26.1 (2023-09-15)

Expand Down
925 changes: 492 additions & 433 deletions lib/Fmt_ast.ml

Large diffs are not rendered by default.

20 changes: 19 additions & 1 deletion test/passing/dune.inc
Original file line number Diff line number Diff line change
Expand Up @@ -4114,13 +4114,31 @@
(package ocamlformat)
(action (diff tests/obuild.ml.err obuild.ml.stderr)))

(rule
(deps tests/.ocamlformat )
(package ocamlformat)
(action
(with-stdout-to ocp_indent_compat-break_colon_after.ml.stdout
(with-stderr-to ocp_indent_compat-break_colon_after.ml.stderr
(run %{bin:ocamlformat} --margin-check --ocp-indent-compat --break-colon=after %{dep:tests/ocp_indent_compat.ml})))))

(rule
(alias runtest)
(package ocamlformat)
(action (diff tests/ocp_indent_compat-break_colon_after.ml.ref ocp_indent_compat-break_colon_after.ml.stdout)))

(rule
(alias runtest)
(package ocamlformat)
(action (diff tests/ocp_indent_compat-break_colon_after.ml.err ocp_indent_compat-break_colon_after.ml.stderr)))

(rule
(deps tests/.ocamlformat )
(package ocamlformat)
(action
(with-stdout-to ocp_indent_compat.ml.stdout
(with-stderr-to ocp_indent_compat.ml.stderr
(run %{bin:ocamlformat} --margin-check %{dep:tests/ocp_indent_compat.ml})))))
(run %{bin:ocamlformat} --margin-check --ocp-indent-compat --break-colon=before %{dep:tests/ocp_indent_compat.ml})))))

(rule
(alias runtest)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
--ocp-indent-compat
--break-colon=after
93 changes: 93 additions & 0 deletions test/passing/tests/ocp_indent_compat-break_colon_after.ml.ref
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
(* Bad: unboxing the function type *)
external i : (int -> float[@unboxed]) = "i" "i_nat"

module type M = sig
val action : action
(** Formatting action: input type and source, and output destination. *)

val doc_atrs :
(string Location.loc * payload) list
-> (string Location.loc * bool) list option
* (string Location.loc * payload) list

val transl_modtype_longident
(* from Typemod *) :
(Location.t -> Env.t -> Longident.t -> Path.t) ref

val transl_modtype_longident
(* foooooooooo fooooooooooooo foooooooooooo foooooooooooooo
foooooooooooooo foooooooooooo *) :
(Location.t -> Env.t -> Longident.t -> Path.t) ref

val imported_sets_of_closures_table :
Simple_value_approx.function_declarations option Set_of_closures_id.Tbl.t

type 'a option_decl =
names:string list
-> doc:string
-> section:[`Formatting | `Operational]
-> ?allow_inline:bool
-> (config -> 'a -> config)
-> (config -> 'a)
-> 'a t

val select :
(* The fsevents context *)
env
-> (* Additional file descriptor to select for reading *)
?read_fdl:fd_select list
-> (* Additional file descriptor to select for writing *)
?write_fdl:fd_select list
-> (* Timeout...like Unix.select *)
timeout:float
-> (* The callback for file system events *)
(event list -> unit)
-> unit

val f :
x:t
(** an extremely long comment about [x] that does not fit on the same
line with [x] *)
-> unit

val f :
fooooooooooooooooo:
(fooooooooooooooo
-> fooooooooooooooooooo
-> foooooooooooooo
-> foooooooooooooo * fooooooooooooooooo
-> foooooooooooooooo )
(** an extremely long comment about [x] that does not fit on the same
line with [x] *)
-> unit
end

let ssmap :
(module MapT
with type key = string
and type data = string
and type map = SSMap.map )
=
()

let ssmap :
(module MapT
with type key = string
and type data = string
and type map = SSMap.map )
-> unit
=
()

let long_function_name :
type a. a long_long_type -> a -> a -> a -> wrap_wrap_wrap -> unit
=
fun () -> ()

let add_edge target dep =
if target <> dep then (
Hashtbl.replace edges dep
(target :: (try Hashtbl.find edges dep with Not_found -> [])) ;
Hashtbl.replace edge_count target
(1 + try Hashtbl.find edge_count target with Not_found -> 0) ;
if not (Hashtbl.mem edge_count dep) then Hashtbl.add edge_count dep 0 )
72 changes: 7 additions & 65 deletions test/passing/tests/ocp_indent_compat.ml
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
[@@@ocamlformat "ocp-indent-compat=true"]

[@@@ocamlformat "break-colon=before"]

(* Bad: unboxing the function type *)
external i : (int -> float[@unboxed]) = "i" "i_nat"

Expand Down Expand Up @@ -89,64 +85,10 @@ let long_function_name
=
fun () -> ()

[@@@ocamlformat "ocp-indent-compat=false"]

[@@@ocamlformat "break-colon=after"]

module type M = sig
val transl_modtype_longident (* from Typemod *) :
(Location.t -> Env.t -> Longident.t -> Path.t) ref

val transl_modtype_longident
(* foooooooooo fooooooooooooo foooooooooooo foooooooooooooo
foooooooooooooo foooooooooooo *) :
(Location.t -> Env.t -> Longident.t -> Path.t) ref

val imported_sets_of_closures_table :
Simple_value_approx.function_declarations option Set_of_closures_id.Tbl.t

val select :
(* The fsevents context *)
env
-> (* Additional file descriptor to select for reading *)
?read_fdl:fd_select list
-> (* Additional file descriptor to select for writing *)
?write_fdl:fd_select list
-> (* Timeout...like Unix.select *)
timeout:float
-> (* The callback for file system events *)
(event list -> unit)
-> unit

val f :
x:t
(** an extremely long comment about [x] that does not fit on the
same line with [x] *)
-> unit

val f :
fooooooooooooooooo:
( fooooooooooooooo
-> fooooooooooooooooooo
-> foooooooooooooo
-> foooooooooooooo * fooooooooooooooooo
-> foooooooooooooooo )
(** an extremely long comment about [x] that does not fit on the
same line with [x] *)
-> unit
end

let array_fold_transf (f : numbering -> 'a -> numbering * 'b) n (a : 'a array)
: numbering * 'b array =
match Array.length a with 0 -> (n, [||]) | 1 -> x

let to_clambda_function (id, (function_decl : Flambda.function_declaration))
: Clambda.ufunction =
(* All that we need in the environment, for translating one closure from a
closed set of closures, is the substitutions for variables bound to the
various closures in the set. Such closures will always be ... *)
x

let long_function_name :
type a. a long_long_type -> a -> a -> a -> wrap_wrap_wrap -> unit =
fun () -> ()
let add_edge target dep =
if target <> dep then (
Hashtbl.replace edges dep
(target :: (try Hashtbl.find edges dep with Not_found -> [])) ;
Hashtbl.replace edge_count target
(1 + try Hashtbl.find edge_count target with Not_found -> 0) ;
if not (Hashtbl.mem edge_count dep) then Hashtbl.add edge_count dep 0 )
1 change: 0 additions & 1 deletion test/passing/tests/ocp_indent_compat.ml.err
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
Warning: tests/ocp_indent_compat.ml:138 exceeds the margin
2 changes: 2 additions & 0 deletions test/passing/tests/ocp_indent_compat.ml.opts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
--ocp-indent-compat
--break-colon=before

0 comments on commit d664fa2

Please sign in to comment.