Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: respect completion item resolve capability #925

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

- Fix document syncing for ranges that span an entire line (#927)

- Respect the client's completion item resolve capability (#925)

## Features

- Re-enable `ocamlformat-rpc` for code formatting (#920)
Expand Down
41 changes: 30 additions & 11 deletions ocaml-lsp-server/src/compl.ml
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ module Complete_by_prefix = struct
(* Without this field the client is not forced to respect the order
provided by merlin. *)
~sortText:(sortText_of_index idx)
~data:compl_params
?data:compl_params
~textEdit
()

Expand All @@ -158,7 +158,8 @@ module Complete_by_prefix = struct
in
Query_commands.dispatch pipeline complete

let process_dispatch_resp doc pos (completion : Query_protocol.completions) =
let process_dispatch_resp ~resolve doc pos
(completion : Query_protocol.completions) =
let range =
let logical_pos = Position.logical pos in
range_prefix
Expand All @@ -185,23 +186,27 @@ module Complete_by_prefix = struct
[data] field to keep it across [textDocument/completion] and the
following [completionItem/resolve] requests *)
let compl_params =
let textDocument =
TextDocumentIdentifier.create
~uri:(Document.uri (Document.Merlin.to_doc doc))
in
CompletionParams.create ~textDocument ~position:pos ()
|> CompletionParams.yojson_of_t
match resolve with
| false -> None
| true ->
Some
(let textDocument =
TextDocumentIdentifier.create
~uri:(Document.uri (Document.Merlin.to_doc doc))
in
CompletionParams.create ~textDocument ~position:pos ()
|> CompletionParams.yojson_of_t)
in
List.mapi
completion_entries
~f:(completionItem_of_completion_entry ~range ~compl_params)

let complete doc prefix pos =
let complete doc prefix pos ~resolve =
let+ (completion : Query_protocol.completions) =
let logical_pos = Position.logical pos in
Document.Merlin.with_pipeline_exn doc (dispatch_cmd ~prefix logical_pos)
in
process_dispatch_resp doc pos completion
process_dispatch_resp ~resolve doc pos completion
end

module Complete_with_construct = struct
Expand Down Expand Up @@ -259,13 +264,26 @@ let complete (state : State.t)
match Document.kind doc with
| `Other -> Fiber.return None
| `Merlin merlin ->
let resolve =
let capabilities = State.client_capabilities state in
match
let open Option.O in
let* td = capabilities.textDocument in
let* compl = td.completion in
let* item = compl.completionItem in
item.resolveSupport
with
| None -> false
| Some { properties } ->
List.mem properties ~equal:String.equal "documentation"
in
Comment on lines +267 to +279
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to have a module-wide state - a lazy value - that would compute this value once instead of doing it on every completion

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it would make sense. We have a Diganostics.t that stores the results of computing capabilities. It will make sense here too.

let+ items =
let position = Position.logical pos in
let prefix =
prefix_of_position ~short_path:false (Document.source doc) position
in
if not (Typed_hole.can_be_hole prefix) then
Complete_by_prefix.complete merlin prefix pos
Complete_by_prefix.complete merlin prefix pos ~resolve
else
let reindex_sortText completion_items =
List.mapi completion_items ~f:(fun idx (ci : CompletionItem.t) ->
Expand Down Expand Up @@ -298,6 +316,7 @@ let complete (state : State.t)
in
let compl_by_prefix_completionItems =
Complete_by_prefix.process_dispatch_resp
~resolve
merlin
pos
compl_by_prefix_resp
Expand Down