Skip to content

Commit 1a94362

Browse files
authored
Remove side-effects of Fmt.list_k that caused comments to be dropped (#1742)
1 parent 5b2bbe5 commit 1a94362

File tree

2 files changed

+28
-2
lines changed

2 files changed

+28
-2
lines changed

lib/Fmt.ml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,8 @@ let list_fl xs pp =
157157
list_pn xs (fun ~prev x ~next ->
158158
pp ~first:(Option.is_none prev) ~last:(Option.is_none next) x )
159159

160-
let list_k l sep f = List.map l ~f |> List.intersperse ~sep |> sequence
160+
let list_k l sep f =
161+
list_fl l (fun ~first:_ ~last x -> f x $ if last then noop else sep)
161162

162163
let list xs sep pp = list_k xs (fmt sep) pp
163164

test/unit/test_fmt.ml

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,31 @@ let tests_list_pn =
7575
let l = ["a"; "b"; "c"; "d"; "e"] in
7676
Fmt.fmt_if_k false (Fmt.list_pn l pp_spy) ) ]
7777

78+
let tests_list_k =
79+
let test name ~expected ~expected_calls f =
80+
( "list_k: " ^ name
81+
, `Quick
82+
, fun () ->
83+
let calls = ref [] in
84+
let record_call x = calls := x :: !calls in
85+
let pp_spy x = record_call x ; Fmt.str x in
86+
let term = f pp_spy in
87+
let got = eval_fmt term in
88+
Alcotest.check Alcotest.(string) Caml.__LOC__ expected got ;
89+
let got_calls = List.rev !calls in
90+
Alcotest.check
91+
Alcotest.(list string)
92+
Caml.__LOC__ expected_calls got_calls )
93+
in
94+
[ test "evaluation order" ~expected:"a b c d e"
95+
~expected_calls:["a"; "b"; "c"; "d"; "e"] (fun pp_spy ->
96+
let l = ["a"; "b"; "c"; "d"; "e"] in
97+
Fmt.list_k l (Fmt.str " ") pp_spy )
98+
; test "does not call pp if not formatting" ~expected:"" ~expected_calls:[]
99+
(fun pp_spy ->
100+
let l = ["a"; "b"; "c"; "d"; "e"] in
101+
Fmt.fmt_if_k false (Fmt.list_k l (Fmt.str " ") pp_spy) ) ]
102+
78103
let tests_sequence =
79104
let test name term ~expected =
80105
( "sequence: " ^ name
@@ -92,4 +117,4 @@ let tests_sequence =
92117
(Fmt.sequence (List.init 300_000 ~f:(fun _ -> Fmt.noop)))
93118
~expected:"" ]
94119

95-
let tests = tests_lazy @ tests_list_pn @ tests_sequence
120+
let tests = tests_lazy @ tests_list_pn @ tests_list_k @ tests_sequence

0 commit comments

Comments
 (0)