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

Improve formatting of assignment operators with comments #2617

Merged
merged 2 commits into from
Nov 13, 2024
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
2 changes: 1 addition & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ profile. This started with version 0.26.0.

### Fixed

- Fix placement of comments in some cases (#2471, #2503, #2506, #2540, #2541, #2592, @gpetiot, @Julow)
- Fix placement of comments in some cases (#2471, #2503, #2506, #2540, #2541, #2592, #2617, @gpetiot, @Julow)
Some comments were being moved or causing OCamlformat to crash.
OCamlformat refuses to format if a comment would be missing in its output, to avoid loosing code.

Expand Down
16 changes: 10 additions & 6 deletions lib/Fmt_ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2014,15 +2014,16 @@ and fmt_expression c ?(box = true) ?(pro = noop) ?eol ?parens
| Pexp_infix ({txt= ":="; loc}, r, v)
when is_simple c.conf (expression_width c) (sub_exp ~ctx r) ->
let bol_indent = Params.Indent.assignment_operator_bol c.conf in
let has_cmts_before = Cmts.has_before c.cmts loc in
let cmts_before =
let indent, adj =
let indent =
(* Use the same break for comment and operator. Comments are placed
according to indentation. *)
match c.conf.fmt_opts.assignment_operator.v with
| `Begin_line -> (bol_indent, noop)
| `End_line -> (2, cut_break)
| `Begin_line -> bol_indent
| `End_line -> 0
in
Cmts.fmt_before c loc ~pro:(break 1 indent) ~epi:adj ~adj
Cmts.fmt_before c loc ~pro:(break 1 indent) ~epi:noop ~adj:noop
in
let cmts_after = Cmts.fmt_after c loc ~pro:noop ~epi:noop in
pro
Expand All @@ -2034,9 +2035,12 @@ and fmt_expression c ?(box = true) ?(pro = noop) ?eol ?parens
$ break 1 bol_indent $ str ":= " $ cmts_after
$ hvbox 2 (fmt_expression c (sub_exp ~ctx v))
| `End_line ->
let break_infix =
if has_cmts_before then break 1 0 else str " "
in
hvbox 0
( hvbox 0 (fmt_expression c (sub_exp ~ctx r) $ cmts_before)
$ str " :=" )
( fmt_expression c (sub_exp ~ctx r)
$ cmts_before $ break_infix $ str ":=" )
$ break 1 2 $ cmts_after
$ hvbox 2 (fmt_expression c (sub_exp ~ctx v)) ) )
| Pexp_prefix ({txt= ("~-" | "~-." | "~+" | "~+.") as op; loc}, e1) ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ Warning: tests/assignment_operator.ml:47 exceeds the margin
Warning: tests/assignment_operator.ml:52 exceeds the margin
Warning: tests/assignment_operator.ml:59 exceeds the margin
Warning: tests/assignment_operator.ml:60 exceeds the margin
Warning: tests/assignment_operator.ml:65 exceeds the margin
5 changes: 5 additions & 0 deletions test/passing/tests/assignment_operator-op_begin_line.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,8 @@ let _ =
:= (* _________________________________________________________________ *)
(* _________________________________________________________________ *)
1

let _ =
aaaaaaa
(* __________________________________________________________________________________ *)
:= bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
6 changes: 6 additions & 0 deletions test/passing/tests/assignment_operator.ml
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,9 @@ let _ =
let _ =
r (* _________________________________________________________________ *) (* _________________________________________________________________ *) := (* _________________________________________________________________ *) (* _________________________________________________________________ *) 1
;;

let _ =
aaaaaaa
(* __________________________________________________________________________________ *)
:= bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
;;
3 changes: 1 addition & 2 deletions test/passing/tests/assignment_operator.ml.err
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
Warning: tests/assignment_operator.ml:30 exceeds the margin
Warning: tests/assignment_operator.ml:50 exceeds the margin
Warning: tests/assignment_operator.ml:67 exceeds the margin
26 changes: 17 additions & 9 deletions test/passing/tests/assignment_operator.ml.ref
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@ let foo =
foo

let _ =
r (* _________________________________________________________________ *) :=
1
r
(* _________________________________________________________________ *)
:= 1

let _ =
r
(* _________________________________________________________________ *)
(* _________________________________________________________________ *)
:= 1
(* _________________________________________________________________ *)
(* _________________________________________________________________ *)
:= 1

let _ =
r :=
Expand All @@ -48,14 +49,21 @@ let _ =
1

let _ =
r (* _________________________________________________________________ *) :=
r
(* _________________________________________________________________ *)
:=
(* _________________________________________________________________ *) 1

let _ =
r
(* _________________________________________________________________ *)
(* _________________________________________________________________ *)
:=
(* _________________________________________________________________ *)
(* _________________________________________________________________ *)
:=
(* _________________________________________________________________ *)
(* _________________________________________________________________ *)
1

let _ =
aaaaaaa
(* __________________________________________________________________________________ *)
:= bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
Loading