Skip to content

Commit fe00ad7

Browse files
committed
fix(completion): preserve dotted operators
Do not shorten an operator-looking completion prefix as though dots separated a module path. Scan operator suffixes with the operator character set as well, so resolve requests reconstruct dotted operators and return their documentation. Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
1 parent 47a9453 commit fe00ad7

4 files changed

Lines changed: 38 additions & 18 deletions

File tree

CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
- Keep completion sort keys lexically ordered beyond 9,999 items. (#1883, @rgrinberg)
6767
- Replace incorrect identifier suffixes when applying completions. (#1964, fixes #838, @rgrinberg)
6868
- Give synthetic completion items stable ordering metadata. (#1882, @rgrinberg)
69+
- Preserve dotted operators in completion prefixes and resolve requests. (#1890, @rgrinberg)
6970
- Correct code-action ranges after multiline text insertions. (#1748, @rgrinberg)
7071
- Allow clients to add their first workspace folder dynamically. (#1747, @rgrinberg)
7172
- Unregister Dune promotion commands after their diagnostics are cleared. (#1746, @rgrinberg)

ocaml-lsp-server/src/compl.ml

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,19 @@ let completion_kind ~supports_enum_member kind : CompletionItemKind.t option =
2323
| `Type -> Some TypeParameter
2424
;;
2525

26+
let ident_char = function
27+
| 'a' .. 'z' | 'A' .. 'Z' | '0' .. '9' | '\128' .. '\255' | '\'' | '_' -> true
28+
| _ -> false
29+
;;
30+
31+
let path_start_char char =
32+
ident_char char
33+
||
34+
match char with
35+
| '~' | '?' | '`' -> true
36+
| _ -> false
37+
;;
38+
2639
let prefix_of_position ~short_path source position =
2740
match Msource.text source with
2841
| "" -> ""
@@ -45,15 +58,19 @@ let prefix_of_position ~short_path source position =
4558
| ' ' | '\n' | '\r' | '\t' | '\012' -> false
4659
| _ -> true)
4760
in
48-
if short_path
61+
let starts_like_a_path =
62+
(not (String.is_empty reconstructed_prefix))
63+
&& path_start_char reconstructed_prefix.[0]
64+
in
65+
if short_path && starts_like_a_path
4966
then (
5067
match String.split reconstructed_prefix ~on:'.' |> List.last with
5168
| Some s -> s
5269
| None -> reconstructed_prefix)
5370
else reconstructed_prefix
5471
;;
5572

56-
let suffix_of_position source position =
73+
let suffix_of_position ~is_char source position =
5774
match Msource.text source with
5875
| "" -> ""
5976
| text ->
@@ -64,12 +81,8 @@ let suffix_of_position source position =
6481
else (
6582
let from = index in
6683
let len =
67-
let ident_char = function
68-
| 'a' .. 'z' | 'A' .. 'Z' | '0' .. '9' | '\128' .. '\255' | '\'' | '_' -> true
69-
| _ -> false
70-
in
7184
let until =
72-
String.lfindi ~pos:from text ~f:(fun _ c -> not (ident_char c))
85+
String.lfindi ~pos:from text ~f:(fun _ c -> not (is_char c))
7386
|> Option.value ~default:len
7487
in
7588
until - from
@@ -79,7 +92,7 @@ let suffix_of_position source position =
7992

8093
let reconstruct_ident source position =
8194
let prefix = prefix_of_position ~short_path:false source position in
82-
let suffix = suffix_of_position source position in
95+
let suffix = suffix_of_position ~is_char:ident_char source position in
8396
let ident = prefix ^ suffix in
8497
Option.some_if (ident <> "") ident
8598
;;
@@ -100,7 +113,7 @@ let edit_range doc pos =
100113
let suffix =
101114
let text_document = Document.Merlin.to_doc doc |> Document.text_document in
102115
let offset = Text_document.absolute_position text_document pos in
103-
suffix_of_position source (`Offset offset)
116+
suffix_of_position ~is_char:ident_char source (`Offset offset)
104117
in
105118
{ range with end_ = { pos with character = pos.character + String.length suffix } }
106119
;;
@@ -498,18 +511,24 @@ let resolve doc (compl : CompletionItem.t) (resolve : Resolve.t) query_doc ~mark
498511
let position : Position.t = resolve.position in
499512
let logical_position = Position.logical position in
500513
let doc =
514+
let prefix =
515+
prefix_of_position ~short_path:true (Document.Merlin.source doc) logical_position
516+
in
517+
let suffix =
518+
let is_operator =
519+
(not (String.is_empty prefix))
520+
&& String.for_all prefix ~f:Ocaml_operator.is_symbolic_character
521+
in
522+
let is_char =
523+
if is_operator then Ocaml_operator.is_symbolic_character else ident_char
524+
in
525+
suffix_of_position ~is_char (Document.Merlin.source doc) logical_position
526+
in
501527
let complete =
502528
let start =
503-
let prefix =
504-
prefix_of_position
505-
~short_path:true
506-
(Document.Merlin.source doc)
507-
logical_position
508-
in
509529
{ position with character = position.character - String.length prefix }
510530
in
511531
let end_ =
512-
let suffix = suffix_of_position (Document.Merlin.source doc) logical_position in
513532
{ position with character = position.character + String.length suffix }
514533
in
515534
let range = Range.create ~start ~end_ in

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ let _ = 1 >>. 2
226226
Fiber.return ()
227227
in
228228
Helpers.test source req;
229-
[%expect {| { "label": ">>." } |}]
229+
[%expect {| { "documentation": "combine docs", "label": ">>." } |}]
230230
;;
231231

232232
let%expect_test "completion resolve converts UTF-16 positions for Merlin" =

ocaml-lsp-server/test/position_prefix_tests.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ let%expect_test "short path prefix" =
7777

7878
let%expect_test "short path preserves a dotted operator" =
7979
prefix_test ~short_path:true "let _ = 1 +." (`Logical (1, 12));
80-
[%expect ""]
80+
[%expect "+."]
8181
;;
8282

8383
let%expect_test "prefix may contain a Latin-1 identifier" =

0 commit comments

Comments
 (0)