Option to activate code lens does not work reliably

[voodoos]

This issue was reported by @EduardoRFS in #1221 (comment)

Codelens was disabled by default in #1134 and an option was added to enable it back.

This option won't be taken into account when the server is started, one have to toggle it off and on after the server starts to effectively activate codelenses.

[EduardoRFS]

The patch on ocamllabs/vscode-ocaml-platform#1321 seems reasonable, on all editors except vscode it is quite easy to set settings on start.

While implementing configuration fetching would be quite annoying.

[voodoos]

I agree that having the client send the configuration looks like the simplest fix, but it's a shame that the solution would be client-specific. It's very surprising that the protocol does not consider sending configuration as part of the initialization of the server... but I am stuck in my attempt to fix it on the server side right now.

The documentation says, about the Initialized notification that:

The server can use the initialized notification, for example, to dynamically register capabilities. The initialized notification may only be sent once.

I tried to send a WorkspaceConfiguration query after receiving this notification. However I cannot get it to work: the server crashes as soon as I call Server.request and I cannot find any other example were a server-side request is made that need to update the state: it works if it's in a task_if_running but that doesn't allow me to update the state afterward.

@rgrinberg does that mean anything to you ? Is that a limitation of the current implementation or am I doing something wrong ?

let on_notification ... =
  ....
  | Initialized ->
    (* Clients often do not send initial configuration to the server so we
       request it after initialization *)
    let+ state =
      let params =
        ConfigurationParams.create
          ~items:
            ConfigurationItem.
              [ create ~section:"ocaml.server.codelens" ()
              ; create ~section:"ocaml.server.extendedHover" ()
              ]
      in
      let+ result =
        Server.request server (Server_request.WorkspaceConfiguration params)
      in
      let state = Server.state server in
      match result with
      | [ codelens; hover ] ->
        let data = json_decoding_thigns ... in
        { state with
          configuration =
            { state.configuration with data = { codelens; extended_hover } }
        }
      | _ -> state
    in
    state

[EduardoRFS]

@voodoos at least from reading on the spec, it seems like the "client" solution was the old solution, before the 3.6.0 version.

This pull model replaces the old push model were the client signaled configuration change via an event. If the server still needs to react to configuration changes (since the server caches the result of workspace/configuration requests) the server should register for an empty configuration change using the following registration pattern:

But yeah it seems like workspace/configuration will be part of the solution, likely we should do both.

[Khady]

Related to #1160 and ocamllabs/vscode-ocaml-platform#1157 (comment) I believe

[voodoos]

This pull model replaces the old push model were the client signaled configuration change via an event. If the server still needs to react to configuration changes (since the server caches the result of workspace/configuration requests) the server should register for an empty configuration change using the following registration pattern:

I find this spec. comment a bit puzzling: pulling from the server is not the correct option for reacting to client-side config changes, and most servers will still have to listen to didChangeConfiguration. I don't understand why they say that it replaces the pushing, except for our very specific case (startup).