Skip to content

Commit

Permalink
Fix unwanted alignment after comment (#2507)
Browse files Browse the repository at this point in the history
This removes the alignment in expressions like:

    let _ = try_with (fun () -> (* comment before *)
                                a ; b (* after b *) )

Co-authored-by: Guillaume Petiot <guillaume@tarides.com>
  • Loading branch information
Julow and gpetiot authored Jan 16, 2024
1 parent 9534fe5 commit 8418244
Show file tree
Hide file tree
Showing 36 changed files with 338 additions and 74 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ profile. This started with version 0.26.0.
- \* Force a break around comments following an infix operator (fix non-stabilizing comments) (#2478, @gpetiot)
- \* Fix the indentation of tuples in attributes and extensions (#2488, @Julow)
- Fix unstable comment around docked functor argument (#2506, @Julow)
- \* Fix unwanted alignment after comment (#2507, @Julow)

## 0.26.1 (2023-09-15)

Expand Down
4 changes: 2 additions & 2 deletions lib/Cmts.ml
Original file line number Diff line number Diff line change
Expand Up @@ -615,8 +615,8 @@ let fmt_cmts_aux t (conf : Conf.t) cmts ~fmt_code pos =
| _ -> noop ) )

(** Format comments for loc. *)
let fmt_cmts t conf ~fmt_code ?pro ?epi ?(eol = Fmt.fmt "@\n") ?(adj = eol)
found loc pos =
let fmt_cmts t conf ~fmt_code ?pro ?epi ?(eol = Fmt.fmt "@;<1000 0>")
?(adj = eol) found loc pos =
let open Fmt in
match found with
| None | Some [] -> noop
Expand Down
11 changes: 6 additions & 5 deletions lib/Fmt_ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3868,10 +3868,11 @@ and fmt_module c ctx ?rec_ ?epi ?(can_sparse = false) keyword ?(eqty = "=")
~fit:true attrs
in
let intro =
str keyword
$ fmt_extension_suffix c ext
$ fmt_attributes c ~pre:(Break (1, 0)) attrs_before
$ fmt_if rec_flag " rec" $ str " " $ fmt_str_loc_opt c name
hvbox 2
( str keyword
$ fmt_extension_suffix c ext
$ fmt_attributes c ~pre:(Break (1, 0)) attrs_before
$ fmt_if rec_flag " rec" $ fmt "@ " $ fmt_str_loc_opt c name )
in
let compact =
Poly.(c.conf.fmt_opts.let_module.v = `Compact) || not can_sparse
Expand Down Expand Up @@ -4188,7 +4189,7 @@ and fmt_module_expr ?(dock_struct = true) c ({ast= m; _} as xmod) =
| Pmod_unpack (e, ty1, ty2) ->
let package_type sep (lid, cstrs) =
break 1 (Params.Indent.mod_unpack_annot c.conf)
$ hvbox 0
$ hovbox 0
( hovbox 0 (str sep $ fmt_longident_loc c lid)
$ fmt_package_type c ctx cstrs )
in
Expand Down
2 changes: 1 addition & 1 deletion test/passing/tests/comments-no-wrap.ml.err
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Warning: tests/comments.ml:186 exceeds the margin
Warning: tests/comments.ml:190 exceeds the margin
Warning: tests/comments.ml:250 exceeds the margin
Warning: tests/comments.ml:430 exceeds the margin
Warning: tests/comments.ml:434 exceeds the margin
16 changes: 10 additions & 6 deletions test/passing/tests/comments-no-wrap.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,11 @@ let%map
_ =
()

type t = < (* a *)
a: int [@atr] (* b *) ; b: int (* c *) >
type t =
< (* a *)
a: int [@atr] (* b *)
; b: int
(* c *) >

type t = < a: int (* a *) ; (* b *) .. (* c *) >

Expand All @@ -277,8 +280,10 @@ let _ =
| None -> do_something ()
| Some _ -> () (* do nothing *) )

let _ = try_with (fun () -> (* comment before *)
a ; b (* after b *) )
let _ =
try_with (fun () ->
(* comment before *)
a ; b (* after b *) )

let _ =
match x with
Expand All @@ -296,8 +301,7 @@ type t =

type t =
(* | B *)
| A
(* A *)
| A (* A *)
| C

type t =
Expand Down
16 changes: 10 additions & 6 deletions test/passing/tests/comments.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,11 @@ let%map
_ =
()

type t = < (* a *)
a: int [@atr] (* b *) ; b: int (* c *) >
type t =
< (* a *)
a: int [@atr] (* b *)
; b: int
(* c *) >

type t = < a: int (* a *) ; (* b *) .. (* c *) >

Expand All @@ -279,8 +282,10 @@ let _ =
| None -> do_something ()
| Some _ -> () (* do nothing *) )

let _ = try_with (fun () -> (* comment before *)
a ; b (* after b *) )
let _ =
try_with (fun () ->
(* comment before *)
a ; b (* after b *) )

let _ =
match x with
Expand All @@ -298,8 +303,7 @@ type t =

type t =
(* | B *)
| A
(* A *)
| A (* A *)
| C

type t =
Expand Down
3 changes: 2 additions & 1 deletion test/passing/tests/comments_args.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ let emit_wrapper_function =
false (* is_pair_generator *)
hoisted true (* no_injection *)
true (* inout_wrapper *)
is_interceptable false (* is_memoize_impl *)
is_interceptable false
(* is_memoize_impl *)
Rx.NonRx false

[@@@ocamlformat "wrap-fun-args=false"]
Expand Down
5 changes: 4 additions & 1 deletion test/passing/tests/extensions-indent.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ let _ = try%lwt Lwt.return 2 with _ -> assert false

let _ =
(* foooooooooooo *)
try%lwt (* fooooooooooo *) Lwt.return 2 with _ -> assert false
try%lwt
(* fooooooooooo *)
Lwt.return 2
with _ -> assert false

let _ =
try%lwt
Expand Down
5 changes: 4 additions & 1 deletion test/passing/tests/extensions.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ let _ = try%lwt Lwt.return 2 with _ -> assert false

let _ =
(* foooooooooooo *)
try%lwt (* fooooooooooo *) Lwt.return 2 with _ -> assert false
try%lwt
(* fooooooooooo *)
Lwt.return 2
with _ -> assert false

let _ =
try%lwt
Expand Down
7 changes: 5 additions & 2 deletions test/passing/tests/extensions_exp_grouping.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,8 @@ let a =
(* test *)
Lwt.return () ;%lwt Lwt.return 1

let a = f ((* test *)
Lwt.return () ;%lwt Lwt.return 1)
let a =
f
( (* test *)
Lwt.return () ;%lwt
Lwt.return 1 )
17 changes: 15 additions & 2 deletions test/passing/tests/ite-compact.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,11 @@ let foo =
let foo = if cmp < 0 then (* foo *) a + b else (* foo *) a - b

let foo =
if cmp < 0 then (* ast higher precedence than context: no parens *)
if cmp < 0 then
(* ast higher precedence than context: no parens *)
false
else if cmp > 0 then (* context higher prec than ast: add parens *)
else if cmp > 0 then
(* context higher prec than ast: add parens *)
true
else if Poly.(assoc_of_prec prec_ast = which_child && which_child <> Non)
then foo
Expand All @@ -137,3 +139,14 @@ let _ =
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz
else fun xxxxxxxxxxxxxxxxx yyyyyyyyyyyyyyyyyyyy zzzzzzzzzzz ->
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz

let _ =
if
(* foo *)
foo
then 0
else if
(* bar *)
bar
then 1
else 2
17 changes: 15 additions & 2 deletions test/passing/tests/ite-compact_closing.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,11 @@ let foo =
let foo = if cmp < 0 then (* foo *) a + b else (* foo *) a - b

let foo =
if cmp < 0 then (* ast higher precedence than context: no parens *)
if cmp < 0 then
(* ast higher precedence than context: no parens *)
false
else if cmp > 0 then (* context higher prec than ast: add parens *)
else if cmp > 0 then
(* context higher prec than ast: add parens *)
true
else if Poly.(assoc_of_prec prec_ast = which_child && which_child <> Non)
then foo
Expand All @@ -152,3 +154,14 @@ let _ =
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz
else fun xxxxxxxxxxxxxxxxx yyyyyyyyyyyyyyyyyyyy zzzzzzzzzzz ->
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz

let _ =
if
(* foo *)
foo
then 0
else if
(* bar *)
bar
then 1
else 2
14 changes: 14 additions & 0 deletions test/passing/tests/ite-fit_or_vertical.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,17 @@ let _ =
else
fun xxxxxxxxxxxxxxxxx yyyyyyyyyyyyyyyyyyyy zzzzzzzzzzz ->
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz

let _ =
if
(* foo *)
foo
then
0
else if
(* bar *)
bar
then
1
else
2
14 changes: 14 additions & 0 deletions test/passing/tests/ite-fit_or_vertical_closing.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,17 @@ let _ =
else
fun xxxxxxxxxxxxxxxxx yyyyyyyyyyyyyyyyyyyy zzzzzzzzzzz ->
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz

let _ =
if
(* foo *)
foo
then
0
else if
(* bar *)
bar
then
1
else
2
14 changes: 14 additions & 0 deletions test/passing/tests/ite-fit_or_vertical_no_indicate.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,17 @@ let _ =
else
fun xxxxxxxxxxxxxxxxx yyyyyyyyyyyyyyyyyyyy zzzzzzzzzzz ->
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz

let _ =
if
(* foo *)
foo
then
0
else if
(* bar *)
bar
then
1
else
2
14 changes: 14 additions & 0 deletions test/passing/tests/ite-kr.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,17 @@ let _ =
else
fun xxxxxxxxxxxxxxxxx yyyyyyyyyyyyyyyyyyyy zzzzzzzzzzz ->
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz

let _ =
if
(* foo *)
foo
then
0
else if
(* bar *)
bar
then
1
else
2
14 changes: 14 additions & 0 deletions test/passing/tests/ite-kr_closing.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,17 @@ let _ =
else
fun xxxxxxxxxxxxxxxxx yyyyyyyyyyyyyyyyyyyy zzzzzzzzzzz ->
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz

let _ =
if
(* foo *)
foo
then
0
else if
(* bar *)
bar
then
1
else
2
15 changes: 13 additions & 2 deletions test/passing/tests/ite-kw_first.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,12 @@ let foo = if cmp < 0 then (* foo *) a + b else (* foo *) a - b

let foo =
if cmp < 0
then (* ast higher precedence than context: no parens *)
then
(* ast higher precedence than context: no parens *)
false
else if cmp > 0
then (* context higher prec than ast: add parens *)
then
(* context higher prec than ast: add parens *)
true
else if Poly.(assoc_of_prec prec_ast = which_child && which_child <> Non)
then foo
Expand All @@ -160,3 +162,12 @@ let _ =
else
fun xxxxxxxxxxxxxxxxx yyyyyyyyyyyyyyyyyyyy zzzzzzzzzzz ->
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz

let _ =
if (* foo *)
foo
then 0
else if (* bar *)
bar
then 1
else 2
15 changes: 13 additions & 2 deletions test/passing/tests/ite-kw_first_closing.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,12 @@ let foo = if cmp < 0 then (* foo *) a + b else (* foo *) a - b

let foo =
if cmp < 0
then (* ast higher precedence than context: no parens *)
then
(* ast higher precedence than context: no parens *)
false
else if cmp > 0
then (* context higher prec than ast: add parens *)
then
(* context higher prec than ast: add parens *)
true
else if Poly.(assoc_of_prec prec_ast = which_child && which_child <> Non)
then foo
Expand All @@ -175,3 +177,12 @@ let _ =
else
fun xxxxxxxxxxxxxxxxx yyyyyyyyyyyyyyyyyyyy zzzzzzzzzzz ->
xxxxxxxxx yyyyyyyyyy zzzzzzzzzzzz

let _ =
if (* foo *)
foo
then 0
else if (* bar *)
bar
then 1
else 2
Loading

0 comments on commit 8418244

Please sign in to comment.