From 6d6e6eeb604c36101ce7141c8e4ad59ef735c195 Mon Sep 17 00:00:00 2001 From: Jules Aguillon Date: Wed, 13 Nov 2024 11:29:50 +0100 Subject: [PATCH 1/2] Improve formatting of assignment operators with comments Avoids outputting the assignment operator on the right of a long comment and removes the inconsistent 1-indent of the operator in some cases: let _ = - r (* _________________________________________________________________ *) := - 1 + r + (* _________________________________________________________________ *) + := 1 let _ = r - (* _________________________________________________________________ *) - (* _________________________________________________________________ *) - := 1 + (* _________________________________________________________________ *) + (* _________________________________________________________________ *) + := 1 Also fix an unstable comment in this case: let _ = aaaaaaa (* __________________________________________________________________________________ *) := bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb Which now formats the same way as other infix operators: let _ = aaaaaaa (* __________________________________________________________________________________ *) <> bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb --- lib/Fmt_ast.ml | 16 +++++++----- .../assignment_operator-op_begin_line.ml.err | 1 + .../assignment_operator-op_begin_line.ml.ref | 5 ++++ test/passing/tests/assignment_operator.ml | 6 +++++ test/passing/tests/assignment_operator.ml.err | 3 +-- test/passing/tests/assignment_operator.ml.ref | 26 ++++++++++++------- 6 files changed, 40 insertions(+), 17 deletions(-) diff --git a/lib/Fmt_ast.ml b/lib/Fmt_ast.ml index 22a06bd2ba..f7b8d7008f 100644 --- a/lib/Fmt_ast.ml +++ b/lib/Fmt_ast.ml @@ -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 @@ -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) -> diff --git a/test/passing/tests/assignment_operator-op_begin_line.ml.err b/test/passing/tests/assignment_operator-op_begin_line.ml.err index ef0999424f..fd05091244 100644 --- a/test/passing/tests/assignment_operator-op_begin_line.ml.err +++ b/test/passing/tests/assignment_operator-op_begin_line.ml.err @@ -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 diff --git a/test/passing/tests/assignment_operator-op_begin_line.ml.ref b/test/passing/tests/assignment_operator-op_begin_line.ml.ref index 1991b684bb..4bc3285fa1 100644 --- a/test/passing/tests/assignment_operator-op_begin_line.ml.ref +++ b/test/passing/tests/assignment_operator-op_begin_line.ml.ref @@ -60,3 +60,8 @@ let _ = := (* _________________________________________________________________ *) (* _________________________________________________________________ *) 1 + +let _ = + aaaaaaa + (* __________________________________________________________________________________ *) + := bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb diff --git a/test/passing/tests/assignment_operator.ml b/test/passing/tests/assignment_operator.ml index 7f390188a9..b14764a4ab 100644 --- a/test/passing/tests/assignment_operator.ml +++ b/test/passing/tests/assignment_operator.ml @@ -48,3 +48,9 @@ let _ = let _ = r (* _________________________________________________________________ *) (* _________________________________________________________________ *) := (* _________________________________________________________________ *) (* _________________________________________________________________ *) 1 ;; + +let _ = + aaaaaaa + (* __________________________________________________________________________________ *) + := bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +;; diff --git a/test/passing/tests/assignment_operator.ml.err b/test/passing/tests/assignment_operator.ml.err index 0561ca9fce..649d097a0d 100644 --- a/test/passing/tests/assignment_operator.ml.err +++ b/test/passing/tests/assignment_operator.ml.err @@ -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 diff --git a/test/passing/tests/assignment_operator.ml.ref b/test/passing/tests/assignment_operator.ml.ref index 18c4dc1c11..4355d24b38 100644 --- a/test/passing/tests/assignment_operator.ml.ref +++ b/test/passing/tests/assignment_operator.ml.ref @@ -28,14 +28,15 @@ let foo = foo let _ = - r (* _________________________________________________________________ *) := - 1 + r + (* _________________________________________________________________ *) + := 1 let _ = r - (* _________________________________________________________________ *) - (* _________________________________________________________________ *) - := 1 + (* _________________________________________________________________ *) + (* _________________________________________________________________ *) + := 1 let _ = r := @@ -48,14 +49,21 @@ let _ = 1 let _ = - r (* _________________________________________________________________ *) := + r + (* _________________________________________________________________ *) + := (* _________________________________________________________________ *) 1 let _ = r - (* _________________________________________________________________ *) - (* _________________________________________________________________ *) - := + (* _________________________________________________________________ *) + (* _________________________________________________________________ *) + := (* _________________________________________________________________ *) (* _________________________________________________________________ *) 1 + +let _ = + aaaaaaa + (* __________________________________________________________________________________ *) + := bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb From 49850081f10ad6dfbe711cfb58e7b1f122963e3f Mon Sep 17 00:00:00 2001 From: Jules Aguillon Date: Wed, 13 Nov 2024 11:36:01 +0100 Subject: [PATCH 2/2] Update CHANGES --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 46a81b49a6..ef6b91ba28 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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.