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

Notify LSP of workspace config when starting #1321

Merged
merged 2 commits into from
Jan 5, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# Unreleased

- Fix direnv compatibility by loading the process.env on every command (#1322)
- Fix server settings missing on LSP startup (#1321)

## 1.14.2

Expand Down
39 changes: 38 additions & 1 deletion src/extension_instance.ml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ type t =
; documentation_server_info : StatusBarItem.t
; sandbox_info : StatusBarItem.t
; ast_editor_state : Ast_editor_state.t
; mutable codelens : bool option
; mutable extended_hover : bool option
}

let sandbox t = t.sandbox
Expand All @@ -23,6 +25,35 @@ let lsp_client t = t.lsp_client

let ocaml_version_exn t = Option.value_exn t.ocaml_version

let send_configuration ?codelens ?extended_hover client =
let codelens =
Option.map codelens ~f:(fun enable ->
LanguageClient.OcamllspSettingEnable.create ~enable ())
in
let extendedHover =
Option.map extended_hover ~f:(fun enable ->
LanguageClient.OcamllspSettingEnable.create ~enable ())
in
let settings =
LanguageClient.OcamllspSettings.create ?codelens ?extendedHover ()
in
let payload =
let settings = LanguageClient.DidChangeConfiguration.create ~settings () in
LanguageClient.DidChangeConfiguration.t_to_js settings
in
LanguageClient.sendNotification
client
"workspace/didChangeConfiguration"
payload

let set_configuration t ?codelens ?extended_hover () =
t.codelens <- codelens;
t.extended_hover <- extended_hover;
match t.lsp_client with
| None -> ()
| Some (client, (_ : Ocaml_lsp.t)) ->
send_configuration ?codelens ?extended_hover client

let stop_server t =
match t.lsp_client with
| None -> Promise.return ()
Expand Down Expand Up @@ -133,6 +164,10 @@ end = struct
with
| Ok () -> ()
| Error (`Msg m) -> show_message `Warn "%s" m);
send_configuration
client
?codelens:t.codelens
?extended_hover:t.extended_hover;
Ok ()
in
match res with
Expand Down Expand Up @@ -189,7 +224,7 @@ end = struct
StatusBarItem.set_text sandbox_info status_bar_item_text
end

let make () =
let make ?codelens ?extended_hover () =
let sandbox = Sandbox.Global in
let sandbox_info = Sandbox_info.make sandbox in
let documentation_server_info = documentation_server_info () in
Expand All @@ -202,6 +237,8 @@ let make () =
; ocaml_version = None
; ast_editor_state
; documentation_server = None
; codelens
; extended_hover
}

let set_documentation_context ~running =
Expand Down
5 changes: 4 additions & 1 deletion src/extension_instance.mli
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ open Import

type t

val make : unit -> t
val make : ?codelens:bool -> ?extended_hover:bool -> unit -> t
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't use optional arguments unless there's a specific reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, was just copying the pattern from somewhere else in the codebase.

LanguageClient.OcamllspSettings.create

But now that I think about it, this is a binding and that's likely the reason why it is using optional parameters.

Copy link
Contributor

Choose a reason for hiding this comment

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

LanguageClient.OcamllspSettings.create

That one is weird as well. It's our own private API, what's it doing inside the vscode API bindings?

But now that I think about it, this is a binding and that's likely the reason why it is using optional parameters.

What do you mean by "binding"?


val sandbox : t -> Sandbox.t

Expand All @@ -23,6 +23,9 @@ val ocaml_version_exn : t -> Ocaml_version.t

val start_language_server : t -> unit Promise.t

val set_configuration :
t -> ?codelens:bool -> ?extended_hover:bool -> unit -> unit
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto


val open_terminal : Sandbox.t -> unit

val disposable : t -> Disposable.t
Expand Down
31 changes: 3 additions & 28 deletions src/vscode_ocaml_platform.ml
Original file line number Diff line number Diff line change
Expand Up @@ -23,34 +23,9 @@ let suggest_to_pick_sandbox () =
let notify_configuration_changes instance =
Workspace.onDidChangeConfiguration
~listener:(fun _event ->
match Extension_instance.language_client instance with
| None -> ()
| Some client ->
let codelens =
Option.map
Settings.(get server_codelens_setting)
~f:(fun enable ->
LanguageClient.OcamllspSettingEnable.create ~enable ())
in
let extendedHover =
Option.map
Settings.(get server_extendedHover_setting)
~f:(fun enable ->
LanguageClient.OcamllspSettingEnable.create ~enable ())
in
let settings =
LanguageClient.OcamllspSettings.create ?codelens ?extendedHover ()
in
let payload =
let settings =
LanguageClient.DidChangeConfiguration.create ~settings ()
in
LanguageClient.DidChangeConfiguration.t_to_js settings
in
LanguageClient.sendNotification
client
"workspace/didChangeConfiguration"
payload)
let codelens = Settings.(get server_codelens_setting) in
let extended_hover = Settings.(get server_extendedHover_setting) in
Extension_instance.set_configuration instance ?codelens ?extended_hover ())
()

let activate (extension : ExtensionContext.t) =
Expand Down