From 6e7ccf8d47a99dd0e70373fcc850ef83fe200c3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Mon, 20 Apr 2026 21:36:07 +0200 Subject: [PATCH 1/4] Fix instability with if-then-else=fit-or-vertical and comments after then When using if-then-else=fit-or-vertical, a comment placed between the `then` keyword and the branch expression caused an infinite oscillation between two formats: then (* comment *) <--> then expr (* comment *) expr The root cause is that the hovbox's soft break (break 1 2) in branch_pro allows the comment to fit on the same line as `then` when the hovbox doesn't break. On re-parsing, the comment is then classified as 'after the then keyword' instead of 'before the expression', producing a different layout that breaks back to the other form. Fix: in the Fit_or_vertical case, when comments exist before the branch body and the overall expression is multi-line, force a hard line break (break 1000 2) so the comment always stays on its own indented line. The single-line check ensures short expressions that fit on one line are unaffected. --- lib/Fmt_ast.ml | 4 ++++ lib/Params.ml | 10 ++++++++-- lib/Params.mli | 1 + 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/Fmt_ast.ml b/lib/Fmt_ast.ml index 62ecceced4..17762a817a 100644 --- a/lib/Fmt_ast.ml +++ b/lib/Fmt_ast.ml @@ -2571,6 +2571,9 @@ and fmt_expression c ?(box = true) ?(pro = noop) ?eol ?parens c loc ) else None in + let has_cmts_before_body = + Cmts.has_before c.cmts xbch.ast.pexp_loc + in let p = Params.get_if_then_else c.conf ~cmts_before_opt ~pro:(fmt_if first pro_inner) ~first ~last @@ -2581,6 +2584,7 @@ and fmt_expression c ?(box = true) ?(pro = noop) ?eol ?parens ~infix_ext_attrs ~fmt_cond:(fmt_expression ~box:false c) ~cmts_before_kw ~cmts_after_kw + ~has_cmts_before_body in let wrap_beginend = match p.beginend_loc with diff --git a/lib/Params.ml b/lib/Params.ml index 9e17d9fb9b..9ecb8471b0 100644 --- a/lib/Params.ml +++ b/lib/Params.ml @@ -849,7 +849,8 @@ type if_then_else = let get_if_then_else (c : Conf.t) ~cmts_before_opt ~pro ~first ~last ~parens_bch ~parens_prev_bch ~xcond ~xbch ~expr_loc ~fmt_infix_ext_attrs - ~infix_ext_attrs ~fmt_cond ~cmts_before_kw ~cmts_after_kw = + ~infix_ext_attrs ~fmt_cond ~cmts_before_kw ~cmts_after_kw + ~has_cmts_before_body = let imd = c.fmt_opts.indicate_multiline_delimiters.v in let beginend_loc, infix_ext_attrs_beginend, branch_expr = let ast = xbch.Ast.ast in @@ -959,7 +960,12 @@ let get_if_then_else (c : Conf.t) ~cmts_before_opt ~pro ~first ~last | _ -> 0 ) ; cond= cond () ; box_keyword_and_expr= Fn.id - ; branch_pro= branch_pro ~begin_end_offset:0 () + ; branch_pro= + ( if + has_cmts_before_body && (not has_cmts_after_kw) + && not (Location.is_single_line expr_loc c.fmt_opts.margin.v) + then break 1000 2 + else branch_pro ~begin_end_offset:0 () ) ; wrap_parens= wrap_parens ~wrap_breaks: diff --git a/lib/Params.mli b/lib/Params.mli index c79466df44..69776b7a2f 100644 --- a/lib/Params.mli +++ b/lib/Params.mli @@ -234,6 +234,7 @@ val get_if_then_else : -> fmt_cond:(expression Ast.xt -> Fmt.t) -> cmts_before_kw:Fmt.t -> cmts_after_kw:Fmt.t option + -> has_cmts_before_body:bool -> if_then_else (** [cmts_before_opt] return the comment before the given location with no breaks around it. *) From 4171f61fb51be7e5b601ccc6350a632030cd80e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Mon, 20 Apr 2026 21:36:07 +0200 Subject: [PATCH 2/4] Add test for if-then-else comment placement stability Add a test case with a long condition and a comment between `then` and the branch expression. This is the pattern that previously triggered 'formatting did not stabilize after 10 iterations' with if-then-else=fit-or-vertical. --- test/passing/refs.ahrefs/ite-compact.ml.ref | 8 ++++++++ test/passing/refs.ahrefs/ite-compact_closing.ml.ref | 8 ++++++++ test/passing/refs.ahrefs/ite-fit_or_vertical.ml.ref | 9 +++++++++ .../refs.ahrefs/ite-fit_or_vertical_closing.ml.ref | 9 +++++++++ .../refs.ahrefs/ite-fit_or_vertical_no_indicate.ml.ref | 9 +++++++++ test/passing/refs.ahrefs/ite-kr.ml.ref | 9 +++++++++ test/passing/refs.ahrefs/ite-kr_closing.ml.ref | 9 +++++++++ test/passing/refs.ahrefs/ite-kw_first.ml.ref | 8 ++++++++ test/passing/refs.ahrefs/ite-kw_first_closing.ml.ref | 8 ++++++++ .../refs.ahrefs/ite-kw_first_no_indicate.ml.ref | 8 ++++++++ test/passing/refs.ahrefs/ite-no_indicate.ml.ref | 8 ++++++++ test/passing/refs.ahrefs/ite-vertical.ml.ref | 9 +++++++++ test/passing/refs.ahrefs/ite.ml.ref | 8 ++++++++ test/passing/refs.default/ite-compact.ml.ref | 8 ++++++++ test/passing/refs.default/ite-compact_closing.ml.ref | 8 ++++++++ test/passing/refs.default/ite-fit_or_vertical.ml.ref | 9 +++++++++ .../refs.default/ite-fit_or_vertical_closing.ml.ref | 9 +++++++++ .../ite-fit_or_vertical_no_indicate.ml.ref | 9 +++++++++ test/passing/refs.default/ite-kr.ml.ref | 9 +++++++++ test/passing/refs.default/ite-kr_closing.ml.ref | 9 +++++++++ test/passing/refs.default/ite-kw_first.ml.ref | 8 ++++++++ test/passing/refs.default/ite-kw_first_closing.ml.ref | 8 ++++++++ .../refs.default/ite-kw_first_no_indicate.ml.ref | 8 ++++++++ test/passing/refs.default/ite-no_indicate.ml.ref | 8 ++++++++ test/passing/refs.default/ite-vertical.ml.ref | 9 +++++++++ test/passing/refs.default/ite.ml.ref | 8 ++++++++ test/passing/refs.janestreet/ite-compact.ml.ref | 8 ++++++++ .../passing/refs.janestreet/ite-compact_closing.ml.ref | 8 ++++++++ .../passing/refs.janestreet/ite-fit_or_vertical.ml.ref | 9 +++++++++ .../refs.janestreet/ite-fit_or_vertical_closing.ml.ref | 9 +++++++++ .../ite-fit_or_vertical_no_indicate.ml.ref | 9 +++++++++ test/passing/refs.janestreet/ite-kr.ml.ref | 9 +++++++++ test/passing/refs.janestreet/ite-kr_closing.ml.ref | 9 +++++++++ test/passing/refs.janestreet/ite-kw_first.ml.ref | 9 +++++++++ .../refs.janestreet/ite-kw_first_closing.ml.ref | 9 +++++++++ .../refs.janestreet/ite-kw_first_no_indicate.ml.ref | 9 +++++++++ test/passing/refs.janestreet/ite-no_indicate.ml.ref | 8 ++++++++ test/passing/refs.janestreet/ite-vertical.ml.ref | 9 +++++++++ test/passing/refs.janestreet/ite.ml.ref | 8 ++++++++ test/passing/refs.ocamlformat/ite-compact.ml.ref | 8 ++++++++ .../refs.ocamlformat/ite-compact_closing.ml.ref | 8 ++++++++ .../refs.ocamlformat/ite-fit_or_vertical.ml.ref | 9 +++++++++ .../ite-fit_or_vertical_closing.ml.ref | 9 +++++++++ .../ite-fit_or_vertical_no_indicate.ml.ref | 9 +++++++++ test/passing/refs.ocamlformat/ite-kr.ml.ref | 9 +++++++++ test/passing/refs.ocamlformat/ite-kr_closing.ml.ref | 9 +++++++++ test/passing/refs.ocamlformat/ite-kw_first.ml.ref | 8 ++++++++ .../refs.ocamlformat/ite-kw_first_closing.ml.ref | 8 ++++++++ .../refs.ocamlformat/ite-kw_first_no_indicate.ml.ref | 8 ++++++++ test/passing/refs.ocamlformat/ite-no_indicate.ml.ref | 8 ++++++++ test/passing/refs.ocamlformat/ite-vertical.ml.ref | 9 +++++++++ test/passing/refs.ocamlformat/ite.ml.ref | 8 ++++++++ test/passing/tests/ite.ml | 10 ++++++++++ 53 files changed, 453 insertions(+) diff --git a/test/passing/refs.ahrefs/ite-compact.ml.ref b/test/passing/refs.ahrefs/ite-compact.ml.ref index f7708332f3..f7d1e60ed5 100644 --- a/test/passing/refs.ahrefs/ite-compact.ml.ref +++ b/test/passing/refs.ahrefs/ite-compact.ml.ref @@ -182,3 +182,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ahrefs/ite-compact_closing.ml.ref b/test/passing/refs.ahrefs/ite-compact_closing.ml.ref index 9a376c30de..e1412092b0 100644 --- a/test/passing/refs.ahrefs/ite-compact_closing.ml.ref +++ b/test/passing/refs.ahrefs/ite-compact_closing.ml.ref @@ -199,3 +199,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ahrefs/ite-fit_or_vertical.ml.ref b/test/passing/refs.ahrefs/ite-fit_or_vertical.ml.ref index 87615873d3..1fcb10530e 100644 --- a/test/passing/refs.ahrefs/ite-fit_or_vertical.ml.ref +++ b/test/passing/refs.ahrefs/ite-fit_or_vertical.ml.ref @@ -218,3 +218,12 @@ let () = end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ahrefs/ite-fit_or_vertical_closing.ml.ref b/test/passing/refs.ahrefs/ite-fit_or_vertical_closing.ml.ref index d14f4864c9..5443fecd22 100644 --- a/test/passing/refs.ahrefs/ite-fit_or_vertical_closing.ml.ref +++ b/test/passing/refs.ahrefs/ite-fit_or_vertical_closing.ml.ref @@ -227,3 +227,12 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ahrefs/ite-fit_or_vertical_no_indicate.ml.ref b/test/passing/refs.ahrefs/ite-fit_or_vertical_no_indicate.ml.ref index 87615873d3..1fcb10530e 100644 --- a/test/passing/refs.ahrefs/ite-fit_or_vertical_no_indicate.ml.ref +++ b/test/passing/refs.ahrefs/ite-fit_or_vertical_no_indicate.ml.ref @@ -218,3 +218,12 @@ let () = end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ahrefs/ite-kr.ml.ref b/test/passing/refs.ahrefs/ite-kr.ml.ref index 2505a46dd4..685aef63e8 100644 --- a/test/passing/refs.ahrefs/ite-kr.ml.ref +++ b/test/passing/refs.ahrefs/ite-kr.ml.ref @@ -256,3 +256,12 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ahrefs/ite-kr_closing.ml.ref b/test/passing/refs.ahrefs/ite-kr_closing.ml.ref index 85bc9a1201..75d0eca42b 100644 --- a/test/passing/refs.ahrefs/ite-kr_closing.ml.ref +++ b/test/passing/refs.ahrefs/ite-kr_closing.ml.ref @@ -263,3 +263,12 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ahrefs/ite-kw_first.ml.ref b/test/passing/refs.ahrefs/ite-kw_first.ml.ref index cb65e8238b..e1261b23d1 100644 --- a/test/passing/refs.ahrefs/ite-kw_first.ml.ref +++ b/test/passing/refs.ahrefs/ite-kw_first.ml.ref @@ -212,3 +212,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ahrefs/ite-kw_first_closing.ml.ref b/test/passing/refs.ahrefs/ite-kw_first_closing.ml.ref index e017b9cf25..9e4abed590 100644 --- a/test/passing/refs.ahrefs/ite-kw_first_closing.ml.ref +++ b/test/passing/refs.ahrefs/ite-kw_first_closing.ml.ref @@ -229,3 +229,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ahrefs/ite-kw_first_no_indicate.ml.ref b/test/passing/refs.ahrefs/ite-kw_first_no_indicate.ml.ref index cb65e8238b..e1261b23d1 100644 --- a/test/passing/refs.ahrefs/ite-kw_first_no_indicate.ml.ref +++ b/test/passing/refs.ahrefs/ite-kw_first_no_indicate.ml.ref @@ -212,3 +212,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ahrefs/ite-no_indicate.ml.ref b/test/passing/refs.ahrefs/ite-no_indicate.ml.ref index f7708332f3..f7d1e60ed5 100644 --- a/test/passing/refs.ahrefs/ite-no_indicate.ml.ref +++ b/test/passing/refs.ahrefs/ite-no_indicate.ml.ref @@ -182,3 +182,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ahrefs/ite-vertical.ml.ref b/test/passing/refs.ahrefs/ite-vertical.ml.ref index 55f8e2992e..822ac5a071 100644 --- a/test/passing/refs.ahrefs/ite-vertical.ml.ref +++ b/test/passing/refs.ahrefs/ite-vertical.ml.ref @@ -255,3 +255,12 @@ let () = end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ahrefs/ite.ml.ref b/test/passing/refs.ahrefs/ite.ml.ref index f7708332f3..f7d1e60ed5 100644 --- a/test/passing/refs.ahrefs/ite.ml.ref +++ b/test/passing/refs.ahrefs/ite.ml.ref @@ -182,3 +182,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.default/ite-compact.ml.ref b/test/passing/refs.default/ite-compact.ml.ref index 53b25d80d4..4ec071d107 100644 --- a/test/passing/refs.default/ite-compact.ml.ref +++ b/test/passing/refs.default/ite-compact.ml.ref @@ -182,3 +182,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.default/ite-compact_closing.ml.ref b/test/passing/refs.default/ite-compact_closing.ml.ref index 9aafd928eb..bc1e7965aa 100644 --- a/test/passing/refs.default/ite-compact_closing.ml.ref +++ b/test/passing/refs.default/ite-compact_closing.ml.ref @@ -197,3 +197,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.default/ite-fit_or_vertical.ml.ref b/test/passing/refs.default/ite-fit_or_vertical.ml.ref index f0eda5723f..fcda092f0f 100644 --- a/test/passing/refs.default/ite-fit_or_vertical.ml.ref +++ b/test/passing/refs.default/ite-fit_or_vertical.ml.ref @@ -218,3 +218,12 @@ let () = end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.default/ite-fit_or_vertical_closing.ml.ref b/test/passing/refs.default/ite-fit_or_vertical_closing.ml.ref index 3453a4bdf5..e631fc39df 100644 --- a/test/passing/refs.default/ite-fit_or_vertical_closing.ml.ref +++ b/test/passing/refs.default/ite-fit_or_vertical_closing.ml.ref @@ -227,3 +227,12 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.default/ite-fit_or_vertical_no_indicate.ml.ref b/test/passing/refs.default/ite-fit_or_vertical_no_indicate.ml.ref index f0eda5723f..fcda092f0f 100644 --- a/test/passing/refs.default/ite-fit_or_vertical_no_indicate.ml.ref +++ b/test/passing/refs.default/ite-fit_or_vertical_no_indicate.ml.ref @@ -218,3 +218,12 @@ let () = end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.default/ite-kr.ml.ref b/test/passing/refs.default/ite-kr.ml.ref index 421fc9efb4..4759601a6e 100644 --- a/test/passing/refs.default/ite-kr.ml.ref +++ b/test/passing/refs.default/ite-kr.ml.ref @@ -258,3 +258,12 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.default/ite-kr_closing.ml.ref b/test/passing/refs.default/ite-kr_closing.ml.ref index 02fc53c71e..681d4af918 100644 --- a/test/passing/refs.default/ite-kr_closing.ml.ref +++ b/test/passing/refs.default/ite-kr_closing.ml.ref @@ -265,3 +265,12 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.default/ite-kw_first.ml.ref b/test/passing/refs.default/ite-kw_first.ml.ref index 72cc9bb657..192f97b5d8 100644 --- a/test/passing/refs.default/ite-kw_first.ml.ref +++ b/test/passing/refs.default/ite-kw_first.ml.ref @@ -212,3 +212,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.default/ite-kw_first_closing.ml.ref b/test/passing/refs.default/ite-kw_first_closing.ml.ref index f00b84035e..23e023f1e3 100644 --- a/test/passing/refs.default/ite-kw_first_closing.ml.ref +++ b/test/passing/refs.default/ite-kw_first_closing.ml.ref @@ -227,3 +227,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.default/ite-kw_first_no_indicate.ml.ref b/test/passing/refs.default/ite-kw_first_no_indicate.ml.ref index 72cc9bb657..192f97b5d8 100644 --- a/test/passing/refs.default/ite-kw_first_no_indicate.ml.ref +++ b/test/passing/refs.default/ite-kw_first_no_indicate.ml.ref @@ -212,3 +212,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.default/ite-no_indicate.ml.ref b/test/passing/refs.default/ite-no_indicate.ml.ref index 53b25d80d4..4ec071d107 100644 --- a/test/passing/refs.default/ite-no_indicate.ml.ref +++ b/test/passing/refs.default/ite-no_indicate.ml.ref @@ -182,3 +182,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.default/ite-vertical.ml.ref b/test/passing/refs.default/ite-vertical.ml.ref index 4616a4cf80..433765d55f 100644 --- a/test/passing/refs.default/ite-vertical.ml.ref +++ b/test/passing/refs.default/ite-vertical.ml.ref @@ -257,3 +257,12 @@ let () = end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.default/ite.ml.ref b/test/passing/refs.default/ite.ml.ref index 53b25d80d4..4ec071d107 100644 --- a/test/passing/refs.default/ite.ml.ref +++ b/test/passing/refs.default/ite.ml.ref @@ -182,3 +182,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.janestreet/ite-compact.ml.ref b/test/passing/refs.janestreet/ite-compact.ml.ref index e714d357eb..f91d603ad5 100644 --- a/test/passing/refs.janestreet/ite-compact.ml.ref +++ b/test/passing/refs.janestreet/ite-compact.ml.ref @@ -184,3 +184,11 @@ let () = () else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t then + (* comment *) + 1 + else 2 +;; diff --git a/test/passing/refs.janestreet/ite-compact_closing.ml.ref b/test/passing/refs.janestreet/ite-compact_closing.ml.ref index d0ac51d5f8..98453e4cf8 100644 --- a/test/passing/refs.janestreet/ite-compact_closing.ml.ref +++ b/test/passing/refs.janestreet/ite-compact_closing.ml.ref @@ -196,3 +196,11 @@ let () = () else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t then + (* comment *) + 1 + else 2 +;; diff --git a/test/passing/refs.janestreet/ite-fit_or_vertical.ml.ref b/test/passing/refs.janestreet/ite-fit_or_vertical.ml.ref index 0c4b7f4c28..1470538220 100644 --- a/test/passing/refs.janestreet/ite-fit_or_vertical.ml.ref +++ b/test/passing/refs.janestreet/ite-fit_or_vertical.ml.ref @@ -233,3 +233,12 @@ let () = else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t then + (* comment *) + 1 + else + 2 +;; diff --git a/test/passing/refs.janestreet/ite-fit_or_vertical_closing.ml.ref b/test/passing/refs.janestreet/ite-fit_or_vertical_closing.ml.ref index c8940fdc08..d43e4749a9 100644 --- a/test/passing/refs.janestreet/ite-fit_or_vertical_closing.ml.ref +++ b/test/passing/refs.janestreet/ite-fit_or_vertical_closing.ml.ref @@ -243,3 +243,12 @@ let () = else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t then + (* comment *) + 1 + else + 2 +;; diff --git a/test/passing/refs.janestreet/ite-fit_or_vertical_no_indicate.ml.ref b/test/passing/refs.janestreet/ite-fit_or_vertical_no_indicate.ml.ref index 0c4b7f4c28..1470538220 100644 --- a/test/passing/refs.janestreet/ite-fit_or_vertical_no_indicate.ml.ref +++ b/test/passing/refs.janestreet/ite-fit_or_vertical_no_indicate.ml.ref @@ -233,3 +233,12 @@ let () = else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t then + (* comment *) + 1 + else + 2 +;; diff --git a/test/passing/refs.janestreet/ite-kr.ml.ref b/test/passing/refs.janestreet/ite-kr.ml.ref index b197cd5d4d..84af5a5395 100644 --- a/test/passing/refs.janestreet/ite-kr.ml.ref +++ b/test/passing/refs.janestreet/ite-kr.ml.ref @@ -279,3 +279,12 @@ let () = else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t then + (* comment *) + 1 + else + 2 +;; diff --git a/test/passing/refs.janestreet/ite-kr_closing.ml.ref b/test/passing/refs.janestreet/ite-kr_closing.ml.ref index 1a21cd9996..3438032dc4 100644 --- a/test/passing/refs.janestreet/ite-kr_closing.ml.ref +++ b/test/passing/refs.janestreet/ite-kr_closing.ml.ref @@ -286,3 +286,12 @@ let () = else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t then + (* comment *) + 1 + else + 2 +;; diff --git a/test/passing/refs.janestreet/ite-kw_first.ml.ref b/test/passing/refs.janestreet/ite-kw_first.ml.ref index bd7e932b9a..04b5c91d50 100644 --- a/test/passing/refs.janestreet/ite-kw_first.ml.ref +++ b/test/passing/refs.janestreet/ite-kw_first.ml.ref @@ -212,3 +212,12 @@ let () = () else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 +;; diff --git a/test/passing/refs.janestreet/ite-kw_first_closing.ml.ref b/test/passing/refs.janestreet/ite-kw_first_closing.ml.ref index 1618210eec..b96500bfee 100644 --- a/test/passing/refs.janestreet/ite-kw_first_closing.ml.ref +++ b/test/passing/refs.janestreet/ite-kw_first_closing.ml.ref @@ -224,3 +224,12 @@ let () = () else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 +;; diff --git a/test/passing/refs.janestreet/ite-kw_first_no_indicate.ml.ref b/test/passing/refs.janestreet/ite-kw_first_no_indicate.ml.ref index bd7e932b9a..04b5c91d50 100644 --- a/test/passing/refs.janestreet/ite-kw_first_no_indicate.ml.ref +++ b/test/passing/refs.janestreet/ite-kw_first_no_indicate.ml.ref @@ -212,3 +212,12 @@ let () = () else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 +;; diff --git a/test/passing/refs.janestreet/ite-no_indicate.ml.ref b/test/passing/refs.janestreet/ite-no_indicate.ml.ref index e714d357eb..f91d603ad5 100644 --- a/test/passing/refs.janestreet/ite-no_indicate.ml.ref +++ b/test/passing/refs.janestreet/ite-no_indicate.ml.ref @@ -184,3 +184,11 @@ let () = () else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t then + (* comment *) + 1 + else 2 +;; diff --git a/test/passing/refs.janestreet/ite-vertical.ml.ref b/test/passing/refs.janestreet/ite-vertical.ml.ref index 26de990a13..2dff7f809b 100644 --- a/test/passing/refs.janestreet/ite-vertical.ml.ref +++ b/test/passing/refs.janestreet/ite-vertical.ml.ref @@ -277,3 +277,12 @@ let () = else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t then + (* comment *) + 1 + else + 2 +;; diff --git a/test/passing/refs.janestreet/ite.ml.ref b/test/passing/refs.janestreet/ite.ml.ref index e714d357eb..f91d603ad5 100644 --- a/test/passing/refs.janestreet/ite.ml.ref +++ b/test/passing/refs.janestreet/ite.ml.ref @@ -184,3 +184,11 @@ let () = () else () ;; + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t then + (* comment *) + 1 + else 2 +;; diff --git a/test/passing/refs.ocamlformat/ite-compact.ml.ref b/test/passing/refs.ocamlformat/ite-compact.ml.ref index 796c965182..e10edc2904 100644 --- a/test/passing/refs.ocamlformat/ite-compact.ml.ref +++ b/test/passing/refs.ocamlformat/ite-compact.ml.ref @@ -181,3 +181,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ocamlformat/ite-compact_closing.ml.ref b/test/passing/refs.ocamlformat/ite-compact_closing.ml.ref index b9dba83eb0..e0c2cff586 100644 --- a/test/passing/refs.ocamlformat/ite-compact_closing.ml.ref +++ b/test/passing/refs.ocamlformat/ite-compact_closing.ml.ref @@ -191,3 +191,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ocamlformat/ite-fit_or_vertical.ml.ref b/test/passing/refs.ocamlformat/ite-fit_or_vertical.ml.ref index bc04c167a3..0857dcd986 100644 --- a/test/passing/refs.ocamlformat/ite-fit_or_vertical.ml.ref +++ b/test/passing/refs.ocamlformat/ite-fit_or_vertical.ml.ref @@ -217,3 +217,12 @@ let () = end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ocamlformat/ite-fit_or_vertical_closing.ml.ref b/test/passing/refs.ocamlformat/ite-fit_or_vertical_closing.ml.ref index 461bf7172b..c8dc177562 100644 --- a/test/passing/refs.ocamlformat/ite-fit_or_vertical_closing.ml.ref +++ b/test/passing/refs.ocamlformat/ite-fit_or_vertical_closing.ml.ref @@ -222,3 +222,12 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ocamlformat/ite-fit_or_vertical_no_indicate.ml.ref b/test/passing/refs.ocamlformat/ite-fit_or_vertical_no_indicate.ml.ref index 2555a13663..1fc52ec15d 100644 --- a/test/passing/refs.ocamlformat/ite-fit_or_vertical_no_indicate.ml.ref +++ b/test/passing/refs.ocamlformat/ite-fit_or_vertical_no_indicate.ml.ref @@ -214,3 +214,12 @@ let () = end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ocamlformat/ite-kr.ml.ref b/test/passing/refs.ocamlformat/ite-kr.ml.ref index 6919676f9f..d76f6b3004 100644 --- a/test/passing/refs.ocamlformat/ite-kr.ml.ref +++ b/test/passing/refs.ocamlformat/ite-kr.ml.ref @@ -259,3 +259,12 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ocamlformat/ite-kr_closing.ml.ref b/test/passing/refs.ocamlformat/ite-kr_closing.ml.ref index 9c0281b334..60e467d68f 100644 --- a/test/passing/refs.ocamlformat/ite-kr_closing.ml.ref +++ b/test/passing/refs.ocamlformat/ite-kr_closing.ml.ref @@ -263,3 +263,12 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ocamlformat/ite-kw_first.ml.ref b/test/passing/refs.ocamlformat/ite-kw_first.ml.ref index ac88050fef..677eb17efe 100644 --- a/test/passing/refs.ocamlformat/ite-kw_first.ml.ref +++ b/test/passing/refs.ocamlformat/ite-kw_first.ml.ref @@ -210,3 +210,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ocamlformat/ite-kw_first_closing.ml.ref b/test/passing/refs.ocamlformat/ite-kw_first_closing.ml.ref index 316270d731..c11585a12a 100644 --- a/test/passing/refs.ocamlformat/ite-kw_first_closing.ml.ref +++ b/test/passing/refs.ocamlformat/ite-kw_first_closing.ml.ref @@ -220,3 +220,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ocamlformat/ite-kw_first_no_indicate.ml.ref b/test/passing/refs.ocamlformat/ite-kw_first_no_indicate.ml.ref index b4a4605bdc..cbf62e803e 100644 --- a/test/passing/refs.ocamlformat/ite-kw_first_no_indicate.ml.ref +++ b/test/passing/refs.ocamlformat/ite-kw_first_no_indicate.ml.ref @@ -207,3 +207,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ocamlformat/ite-no_indicate.ml.ref b/test/passing/refs.ocamlformat/ite-no_indicate.ml.ref index bdbf91c7ad..705465f2d6 100644 --- a/test/passing/refs.ocamlformat/ite-no_indicate.ml.ref +++ b/test/passing/refs.ocamlformat/ite-no_indicate.ml.ref @@ -178,3 +178,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/refs.ocamlformat/ite-vertical.ml.ref b/test/passing/refs.ocamlformat/ite-vertical.ml.ref index 3aa8a88b70..20f7345a1d 100644 --- a/test/passing/refs.ocamlformat/ite-vertical.ml.ref +++ b/test/passing/refs.ocamlformat/ite-vertical.ml.ref @@ -258,3 +258,12 @@ let () = end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 diff --git a/test/passing/refs.ocamlformat/ite.ml.ref b/test/passing/refs.ocamlformat/ite.ml.ref index 796c965182..e10edc2904 100644 --- a/test/passing/refs.ocamlformat/ite.ml.ref +++ b/test/passing/refs.ocamlformat/ite.ml.ref @@ -181,3 +181,11 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else 2 diff --git a/test/passing/tests/ite.ml b/test/passing/tests/ite.ml index 27e43f5448..8ad3683691 100644 --- a/test/passing/tests/ite.ml +++ b/test/passing/tests/ite.ml @@ -180,3 +180,13 @@ let () = () end else () + +(* Long condition with comment after then keyword *) +let test = + if + long_function_name_to_force_break a b c d e f g h i j k l m n o p q r s t + then + (* comment *) + 1 + else + 2 From 11d5ced39afeee65f2d5269898732717e305318e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonin=20D=C3=A9cimo?= Date: Wed, 22 Apr 2026 12:30:15 +0200 Subject: [PATCH 3/4] Fix instability with if-then-else=fit-or-vertical and comments after then When using if-then-else=fit-or-vertical, a comment placed between the `then` keyword and the branch expression caused an infinite oscillation between two formats: then (* comment *) <--> then expr (* comment *) expr The root cause is that the hovbox's soft break (break 1 2) in branch_pro allows the comment to fit on the same line as `then` when the hovbox doesn't break. On re-parsing, the comment is then classified as 'after the then keyword' instead of 'before the expression', producing a different layout that breaks back to the other form. Fix: in the Fit_or_vertical case, when the expression is multi-line and there is no begin..end or keyword comment, use cmts_before_opt to check for and consume comments before the branch body directly in branch_pro, placing them with a forced line break so the comment stays on its own indented line. The single-line check ensures short expressions that fit on one line are unaffected. --- lib/Fmt_ast.ml | 4 ---- lib/Params.ml | 10 ++++++---- lib/Params.mli | 1 - 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/Fmt_ast.ml b/lib/Fmt_ast.ml index 17762a817a..62ecceced4 100644 --- a/lib/Fmt_ast.ml +++ b/lib/Fmt_ast.ml @@ -2571,9 +2571,6 @@ and fmt_expression c ?(box = true) ?(pro = noop) ?eol ?parens c loc ) else None in - let has_cmts_before_body = - Cmts.has_before c.cmts xbch.ast.pexp_loc - in let p = Params.get_if_then_else c.conf ~cmts_before_opt ~pro:(fmt_if first pro_inner) ~first ~last @@ -2584,7 +2581,6 @@ and fmt_expression c ?(box = true) ?(pro = noop) ?eol ?parens ~infix_ext_attrs ~fmt_cond:(fmt_expression ~box:false c) ~cmts_before_kw ~cmts_after_kw - ~has_cmts_before_body in let wrap_beginend = match p.beginend_loc with diff --git a/lib/Params.ml b/lib/Params.ml index 9ecb8471b0..1b70162b61 100644 --- a/lib/Params.ml +++ b/lib/Params.ml @@ -849,8 +849,7 @@ type if_then_else = let get_if_then_else (c : Conf.t) ~cmts_before_opt ~pro ~first ~last ~parens_bch ~parens_prev_bch ~xcond ~xbch ~expr_loc ~fmt_infix_ext_attrs - ~infix_ext_attrs ~fmt_cond ~cmts_before_kw ~cmts_after_kw - ~has_cmts_before_body = + ~infix_ext_attrs ~fmt_cond ~cmts_before_kw ~cmts_after_kw = let imd = c.fmt_opts.indicate_multiline_delimiters.v in let beginend_loc, infix_ext_attrs_beginend, branch_expr = let ast = xbch.Ast.ast in @@ -962,9 +961,12 @@ let get_if_then_else (c : Conf.t) ~cmts_before_opt ~pro ~first ~last ; box_keyword_and_expr= Fn.id ; branch_pro= ( if - has_cmts_before_body && (not has_cmts_after_kw) + (not has_beginend) && (not has_cmts_after_kw) && not (Location.is_single_line expr_loc c.fmt_opts.margin.v) - then break 1000 2 + then + match cmts_before_opt xbch.ast.pexp_loc with + | Some cmts -> break 1000 2 $ cmts + | None -> branch_pro ~begin_end_offset:0 () else branch_pro ~begin_end_offset:0 () ) ; wrap_parens= wrap_parens diff --git a/lib/Params.mli b/lib/Params.mli index 69776b7a2f..c79466df44 100644 --- a/lib/Params.mli +++ b/lib/Params.mli @@ -234,7 +234,6 @@ val get_if_then_else : -> fmt_cond:(expression Ast.xt -> Fmt.t) -> cmts_before_kw:Fmt.t -> cmts_after_kw:Fmt.t option - -> has_cmts_before_body:bool -> if_then_else (** [cmts_before_opt] return the comment before the given location with no breaks around it. *) From c609e2a0d76fc03274cd4eb4887d773075a3e162 Mon Sep 17 00:00:00 2001 From: Jules Aguillon Date: Wed, 22 Apr 2026 15:15:43 +0200 Subject: [PATCH 4/4] Update CHANGES --- CHANGES.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 0f2a70ab84..75158c9bdd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,13 @@ Items marked with an asterisk (\*) are changes that are likely to format existing code differently from the previous release when using the default profile. This started with version 0.26.0. +## unreleased + +### Fixed + +- Fix instability on long if-then-else with `if-then-else=fit-or-vertical` + (#2797, @MisterDA) + ## 0.29.0 ### Highlight