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

Don't indent attributes after let/val/external #2317

Merged
merged 3 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
- Restore short form formatting of record field aliases (#2282, @gpetiot)
- Tweaks the JaneStreet profile to be more consistent with ocp-indent (#2281, #2284, #2289, #2299, #2302, #2309, #2310, #2311, #2313, @gpetiot, @Julow)
- Improve formatting of class signatures (#2301, @gpetiot, @Julow)
- Don't indent attributes after a let/val/external (#2317, @Julow)

### New features

Expand Down
3 changes: 0 additions & 3 deletions lib/Conf.ml
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ let conventional_profile from =
; space_around_records= elt true
; space_around_variants= elt true
; stritem_extension_indent= elt 0
; stritem_attributes_indent= elt true
; type_decl= elt `Compact
; type_decl_indent= elt 2
; wrap_comments= elt false
Expand Down Expand Up @@ -174,7 +173,6 @@ let ocamlformat_profile from =
; space_around_records= elt false
; space_around_variants= elt false
; stritem_extension_indent= elt 0
; stritem_attributes_indent= elt true
; type_decl= elt `Compact
; type_decl_indent= elt 2
; wrap_comments= elt false
Expand Down Expand Up @@ -241,7 +239,6 @@ let janestreet_profile from =
; space_around_records= elt true
; space_around_variants= elt true
; stritem_extension_indent= elt 2
; stritem_attributes_indent= elt false
; type_decl= elt `Sparse
; type_decl_indent= elt 2
; wrap_comments= elt false
Expand Down
1 change: 0 additions & 1 deletion lib/Conf_t.ml
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ type fmt_opts =
; space_around_records: bool elt
; space_around_variants: bool elt
; stritem_extension_indent: int elt
; stritem_attributes_indent: bool elt
; type_decl: [`Compact | `Sparse] elt
; type_decl_indent: int elt
; wrap_comments: bool elt
Expand Down
1 change: 0 additions & 1 deletion lib/Conf_t.mli
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ type fmt_opts =
; space_around_records: bool elt
; space_around_variants: bool elt
; stritem_extension_indent: int elt
; stritem_attributes_indent: bool elt
; type_decl: [`Compact | `Sparse] elt
; type_decl_indent: int elt
; wrap_comments: bool elt (** Wrap comments at margin. *)
Expand Down
11 changes: 2 additions & 9 deletions lib/Fmt_ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3069,9 +3069,6 @@ and fmt_value_description ?ext c ctx vd =
wrap "{|" "|}" (str s)
else wrap "\"" "\"" (str (String.escaped s))
in
let attrs_indent =
if c.conf.fmt_opts.stritem_attributes_indent.v then 2 else 0
in
hvbox 0
( doc_before
$ box_fun_sig_args c 2
Expand All @@ -3091,7 +3088,7 @@ and fmt_value_description ?ext c ctx vd =
$ fmt_if (not (List.is_empty pval_prim)) "@ = "
$ hvbox_if (List.length pval_prim > 1) 0
@@ list pval_prim "@;" fmt_val_prim )
$ fmt_item_attributes c ~pre:(Break (1, attrs_indent)) atrs
$ fmt_item_attributes c ~pre:(Break (1, 0)) atrs
$ doc_after )

and fmt_tydcl_params c ctx params =
Expand Down Expand Up @@ -4271,11 +4268,7 @@ and fmt_value_binding c ~rec_flag ?ext ?in_ ?epi ctx
, Cmts.fmt_after c lb_loc )
| None ->
let epi =
let indent =
if c.conf.fmt_opts.stritem_attributes_indent.v then indent else 0
in
fmt_item_attributes c ~pre:(Break (1, indent)) at_at_attrs
$ fmt_opt epi
fmt_item_attributes c ~pre:(Break (1, 0)) at_at_attrs $ fmt_opt epi
in
( true
, noop
Expand Down
14 changes: 7 additions & 7 deletions test/passing/tests/attributes.ml
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ type t =
(** docstring that is long enough to break *) }

val foo : int
[@@deprecated "it is good the salad"] [@@warning "-32"] [@@warning "-99"]
[@@deprecated "it is good the salad"] [@@warning "-32"] [@@warning "-99"]

val foo : int
[@@deprecated "it is good the salad"]
[@@warning "-32"]
[@@warning "-99"]
[@@some long comment]
[@@deprecated "it is good the salad"]
[@@warning "-32"]
[@@warning "-99"]
[@@some long comment]

type t = A of int [@attr] | B of (float[@attr]) | C [@attr]

Expand Down Expand Up @@ -423,8 +423,8 @@ let[@a

let raise_length_mismatch name n1 n2 =
invalid_argf "length mismatch in %s: %d <> %d" name n1 n2 ()
[@@cold] [@@inline never] [@@local never] [@@specialise never]
[@@cold] [@@inline never] [@@local never] [@@specialise never]

external unsafe_memset : t -> pos:int -> len:int -> char -> unit
= "bigstring_memset_stub"
[@@noalloc]
[@@noalloc]
4 changes: 2 additions & 2 deletions test/passing/tests/attributes.mli.ref
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
[@@@ocaml.doc "_"]

val f : int -> int -> int
[@@cold] [@@inline never] [@@local never] [@@specialise never]
[@@cold] [@@inline never] [@@local never] [@@specialise never]

external unsafe_memset : t -> pos:int -> len:int -> char -> unit
= "bigstring_memset_stub"
[@@noalloc]
[@@noalloc]
10 changes: 5 additions & 5 deletions test/passing/tests/cases_exp_grouping.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ let _ =
| C -> fooooooooooooo
| D -> fooooooooooooo
end
[@@ocamlformat "break-cases=fit"]
[@@ocamlformat "break-cases=fit"]

let _ =
match x with
Expand All @@ -30,7 +30,7 @@ let _ =
| D ->
fooooooooooooo
end
[@@ocamlformat "break-cases=nested"]
[@@ocamlformat "break-cases=nested"]

let _ =
match x with
Expand All @@ -50,7 +50,7 @@ let _ =
| C -> fooooooooooooo
| D -> fooooooooooooo
end
[@@ocamlformat "break-cases=toplevel"]
[@@ocamlformat "break-cases=toplevel"]

let _ =
match x with
Expand All @@ -70,7 +70,7 @@ let _ =
| C -> fooooooooooooo
| D -> fooooooooooooo
end
[@@ocamlformat "break-cases=fit-or-vertical"]
[@@ocamlformat "break-cases=fit-or-vertical"]

let _ =
match x with
Expand All @@ -90,4 +90,4 @@ let _ =
| C -> fooooooooooooo
| D -> fooooooooooooo
end
[@@ocamlformat "break-cases=all"]
[@@ocamlformat "break-cases=all"]
8 changes: 4 additions & 4 deletions test/passing/tests/expect_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ let%bench "test" = fun () -> ()
let%expect_test _ =
assert false ;
[%expect.unreachable]
[@@expect.uncaught_exn
{|
[@@expect.uncaught_exn
{|
(* CR expect_test_collector: This test expectation appears to contain a backtrace.
This is strongly discouraged as backtraces are fragile.
Please change this test to not include a backtrace. *)
Expand All @@ -18,8 +18,8 @@ let%expect_test _ =
let _ =
assert false ;
[%expect.unreachable]
[@@expect.uncaught_exn
{|
[@@expect.uncaught_exn
{|
"Assert_failure test.ml:5:6"
Raised at file "test.ml", line 4, characters 6-18
Called from file "collector/expect_test_collector.ml", line 225, characters 12-19 |}]
8 changes: 4 additions & 4 deletions test/passing/tests/extensions-indent.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,10 @@ let foo =
fooooooooooooooooooooooooooo foooooooooooooooooooooooooooooooooo
foooooooooooooooooooooooooooooooooo foooooooooooooooooooooooooooo
foooooooooooooooooooooooooooo]
[@@foooooooooo
fooooooooooooooooooooooooooo foooooooooooooooooooooooooooooooooo
foooooooooooooooooooooooooooooooooo foooooooooooooooooooooooooooo
foooooooooooooooooooooooooooo]
[@@foooooooooo
fooooooooooooooooooooooooooo foooooooooooooooooooooooooooooooooo
foooooooooooooooooooooooooooooooooo foooooooooooooooooooooooooooo
foooooooooooooooooooooooooooo]

[%%foooooooooo:
fooooooooooooooooooooooooooo foooooooooooooooooooooooooooooooooo
Expand Down
8 changes: 4 additions & 4 deletions test/passing/tests/extensions.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,10 @@ let foo =
fooooooooooooooooooooooooooo foooooooooooooooooooooooooooooooooo
foooooooooooooooooooooooooooooooooo foooooooooooooooooooooooooooo
foooooooooooooooooooooooooooo]
[@@foooooooooo
fooooooooooooooooooooooooooo foooooooooooooooooooooooooooooooooo
foooooooooooooooooooooooooooooooooo foooooooooooooooooooooooooooo
foooooooooooooooooooooooooooo]
[@@foooooooooo
fooooooooooooooooooooooooooo foooooooooooooooooooooooooooooooooo
foooooooooooooooooooooooooooooooooo foooooooooooooooooooooooooooo
foooooooooooooooooooooooooooo]

[%%foooooooooo:
fooooooooooooooooooooooooooo foooooooooooooooooooooooooooooooooo
Expand Down
10 changes: 5 additions & 5 deletions test/passing/tests/option.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,28 @@ let _ =
else (
something loooooooooooooooooooooooooooooooong enough to_trigger a break ;
this is more )
[@@ocamlformat "if-then-else=keyword-first"]
[@@ocamlformat "if-then-else=keyword-first"]

let _ =
if b then e
else (
something loooooooooooooooooooooooooooooooong enough to_trigger a break ;
this is more )
[@@ocamlformat.typo "if-then-else=keyword-first"]
[@@ocamlformat.typo "if-then-else=keyword-first"]

let _ =
if b then e
else (
something loooooooooooooooooooooooooooooooong enough to_trigger a break ;
this is more )
[@@ocamlformat 1, "if-then-else=keyword-first"]
[@@ocamlformat 1, "if-then-else=keyword-first"]

let _ =
if b then e
else (
something loooooooooooooooooooooooooooooooong enough to_trigger a break ;
this is more )
[@@ocamlformat "if-then-else=bad"]
[@@ocamlformat "if-then-else=bad"]

module M = struct
[@@@ocamlformat "if-then-else=keyword-first"]
Expand All @@ -36,7 +36,7 @@ module M = struct
else (
something loooooooooooooooooooooooooooooooong enough to_trigger a break ;
this is more )
[@@ocamlformat "if-then-else=bad"]
[@@ocamlformat "if-then-else=bad"]

let _ =
if b
Expand Down