Skip to content

Commit d17c679

Browse files
authored
Remove formatting in code action destruct (#1519)
* Add test (that needs parenthesis) * Remove `ocamlformat` application on destruct result * Add CHANGES entry
1 parent 53bbb83 commit d17c679

File tree

3 files changed

+91
-10
lines changed

3 files changed

+91
-10
lines changed

CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
- Support for `class`, `class type`, `method` and `property` for `DocumentSymbol` query (#1487 fixes #1449)
1111
- Fix `inlay-hint` for function parameters (#1515)
1212
- More precise diagnostics in the event of a failed identifier search (`Definition_query`) (#1518)
13+
- Remove `ocamlformat` application after `destruct` (that remove some useful parenthesis) (#1519)
1314

1415
# 1.22.0
1516

ocaml-lsp-server/src/code_actions/action_destruct.ml

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,19 +47,15 @@ let code_action (state : State.t) doc (params : CodeActionParams.t) =
4747
let* res = Document.Merlin.dispatch ~name:"destruct" merlin command in
4848
(match res with
4949
| Ok (loc, newText) ->
50-
let+ newText =
51-
let+ formatted_text =
52-
Ocamlformat_rpc.format_type state.ocamlformat_rpc ~typ:newText
53-
in
54-
match formatted_text with
55-
| Ok formatted_text -> formatted_text
56-
| Error _ -> newText
57-
in
5850
let supportsJumpToNextHole =
5951
State.experimental_client_capabilities state
6052
|> Client.Experimental_capabilities.supportsJumpToNextHole
6153
in
62-
Some (code_action_of_case_analysis ~supportsJumpToNextHole doc uri (loc, newText))
54+
let opt =
55+
Some
56+
(code_action_of_case_analysis ~supportsJumpToNextHole doc uri (loc, newText))
57+
in
58+
Fiber.return opt
6359
| Error
6460
{ exn =
6561
( Merlin_analysis.Destruct.Wrong_parent _

ocaml-lsp-server/test/e2e-new/code_actions.ml

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ let f (x : t) = x
535535
{
536536
"edits": [
537537
{
538-
"newText": "match x with Foo _ -> _ | Bar _ -> _\n",
538+
"newText": "match x with | Foo _ -> _ | Bar _ -> _",
539539
"range": {
540540
"end": { "character": 17, "line": 2 },
541541
"start": { "character": 16, "line": 2 }
@@ -927,6 +927,90 @@ let f (x: q) =
927927
|}]
928928
;;
929929
930+
let%expect_test "can destruct on sub-expression" =
931+
let source =
932+
{ocaml|
933+
let defered_peek x = if x >= 0 then Some (`Foo x) else None
934+
let job_reader = 10
935+
936+
let _ = defered_peek job_reader
937+
|ocaml}
938+
in
939+
let range =
940+
let start = Position.create ~line:4 ~character:8 in
941+
let end_ = Position.create ~line:4 ~character:31 in
942+
Range.create ~start ~end_
943+
in
944+
print_code_actions source range ~filter:(find_action "destruct (enumerate cases)");
945+
[%expect
946+
{|
947+
Code actions:
948+
{
949+
"edit": {
950+
"documentChanges": [
951+
{
952+
"edits": [
953+
{
954+
"newText": "match defered_peek job_reader with | None -> _ | Some _ -> _",
955+
"range": {
956+
"end": { "character": 31, "line": 4 },
957+
"start": { "character": 8, "line": 4 }
958+
}
959+
}
960+
],
961+
"textDocument": { "uri": "file:///foo.ml", "version": 0 }
962+
}
963+
]
964+
},
965+
"isPreferred": false,
966+
"kind": "destruct (enumerate cases)",
967+
"title": "Destruct (enumerate cases)"
968+
}
969+
|}]
970+
;;
971+
972+
let%expect_test "can destruct on sub-expression that need parenthesis" =
973+
let source =
974+
{ocaml|
975+
let defered_peek x = if x >= 0 then Some (`Foo x) else None
976+
let job_reader = 10
977+
978+
let _ = defered_peek job_reader
979+
|ocaml}
980+
in
981+
let range =
982+
let start = Position.create ~line:4 ~character:21 in
983+
let end_ = Position.create ~line:4 ~character:31 in
984+
Range.create ~start ~end_
985+
in
986+
print_code_actions source range ~filter:(find_action "destruct (enumerate cases)");
987+
[%expect
988+
{|
989+
Code actions:
990+
{
991+
"edit": {
992+
"documentChanges": [
993+
{
994+
"edits": [
995+
{
996+
"newText": "(match job_reader with | 0 -> _ | _ -> _)",
997+
"range": {
998+
"end": { "character": 31, "line": 4 },
999+
"start": { "character": 21, "line": 4 }
1000+
}
1001+
}
1002+
],
1003+
"textDocument": { "uri": "file:///foo.ml", "version": 0 }
1004+
}
1005+
]
1006+
},
1007+
"isPreferred": false,
1008+
"kind": "destruct (enumerate cases)",
1009+
"title": "Destruct (enumerate cases)"
1010+
}
1011+
|}]
1012+
;;
1013+
9301014
let%expect_test "can infer module interfaces" =
9311015
let impl_source =
9321016
{ocaml|

0 commit comments

Comments
 (0)