Skip to content

Commit

Permalink
fix: remove intrusive version checking
Browse files Browse the repository at this point in the history
Only notify users when all of the following conditions hold:

* The running version of ocamllsp is "official". We don't offer
  suggestions for development versions.

* We know something about the user's version ocamllsp. If the version
  isn't recognized by us, we do not second guess the user.

* We are certain the user is able to upgrade to a newer version. If the
  user is forced to an older version of lsp, we allow him to use the
  plugin without notifications.

ps-id: FC109E94-B57C-4226-9AAE-512F45524A91
  • Loading branch information
rgrinberg committed Feb 14, 2022
1 parent 3149514 commit bcaf28a
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 69 deletions.
5 changes: 5 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Change Log

## 1.9.1

- Make the check for out of date ocamllsp more conservative. It will no longer
alert the user unless the extension is certain an upgrade is possible (#859)

## 1.9.0

- Dune syntax highlighting fix (#742)
Expand Down
28 changes: 9 additions & 19 deletions src/extension_commands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -158,25 +158,15 @@ end = struct
(** Shows appropriate message for when the [ocaml-lsp] in use by the extension
doesn't support jumping to holes. *)
let ocaml_lsp_doesn't_support_holes instance ocaml_lsp =
let suggestion =
match
Ocaml_lsp.is_version_up_to_date ocaml_lsp
(Extension_instance.ocaml_version_exn instance)
with
| Ok is_up_to_date ->
if is_up_to_date then
(* ocamllsp is "up-to-date", so user needs newer ocaml to get newer
ocamllsp, which supports typed holes *)
"The extension requires a newer version of `ocamllsp`, which needs a \
new version of OCaml. Please, consider upgrading the OCaml version \
used in this sandbox."
else "Consider upgrading the package `ocaml-lsp-server`."
| Error (`Msg m) ->
sprintf "There is something wrong with your `ocamllsp`. Error: %s" m
in
show_message `Warn
"The installed version of `ocamllsp` does not support typed holes. %s"
suggestion
match
Ocaml_lsp.is_version_up_to_date ocaml_lsp
(Extension_instance.ocaml_version_exn instance)
with
| Ok () -> ()
| Error (`Msg msg) ->
show_message `Warn
"The installed version of `ocamllsp` does not support typed holes. %s"
msg

let current_cursor_pos text_editor =
let selection = TextEditor.selection text_editor in
Expand Down
7 changes: 1 addition & 6 deletions src/extension_instance.ml
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,7 @@ end = struct
(match
Ocaml_lsp.is_version_up_to_date ocaml_lsp (ocaml_version_exn t)
with
| Ok is_up_to_date ->
if not is_up_to_date then
show_message `Warn
"The installed version of `ocamllsp` is out of date. You may be \
missing out on cool features or important bug fixes. Consider \
upgrading the package `ocaml-lsp-server`."
| Ok () -> ()
| Error (`Msg m) -> show_message `Warn "%s" m);
Ok ()
in
Expand Down
98 changes: 55 additions & 43 deletions src/ocaml_lsp.ml
Original file line number Diff line number Diff line change
Expand Up @@ -53,64 +53,76 @@ type t =

let get_version_from_serverInfo { serverInfo; experimental_capabilities = _ } =
match serverInfo with
| None -> Error `Missing_serverInfo (* ocamllsp should have [serverInfo] *)
| None -> None
| Some { name; version } ->
if String.equal name "ocamllsp" then
match version with
| None -> Error `ServerInfo_version_missing
| Some v -> Ok v
if String.equal name "ocamllsp" then version
else (
log_chan ~section:"Ocaml_lsp.get_version" `Warn
"the language server is not ocamllsp";
(* practically impossible but let's be defensive *)
Error `Language_server_isn't_ocamllsp)
None)

let get_version_semver t =
match get_version_from_serverInfo t with
| Ok v -> (
match String.split v ~on:'-' |> List.hd with
| Some v -> Ok v
| None -> Error `Unable_to_parse_version)
| Error _ as err -> err
let lsp_versions =
let main =
[ ( (4, 12)
, [| "1.5.0"
; "1.6.0"
; "1.6.1"
; "1.7.0"
; "1.8.0"
; "1.8.1"
; "1.8.2"
; "1.8.3"
; "1.9.0"
|] )
; ((4, 13), [| "1.9.1"; "1.10.0"; "1.10.1" |])
]
in
let rest =
let lsp = [| "1.0.0"; "1.1.0"; "1.2.0"; "1.3.0"; "1.4.0"; "1.4.1" |] in
List.range ~start:`inclusive ~stop:`inclusive 6 11
|> List.map ~f:(fun minor -> ((4, minor), lsp))
in
main @ rest

let available_versions version =
let prefix = (Ocaml_version.major version, Ocaml_version.minor version) in
List.Assoc.find lsp_versions ~equal:Poly.equal prefix

let is_version_up_to_date t ocaml_v =
let ocamllsp_version = get_version_semver t in
let ocamllsp_version = get_version_from_serverInfo t in
let res =
(* if the server doesn't have a version, we assume it's ancient and we can
offer an upgrade *)
match ocamllsp_version with
| Ok v -> (
match ocaml_v with
| _ when Ocaml_version.(ocaml_v < Releases.v4_06_0) ->
Error (`Ocaml_version_not_supported ocaml_v)
| _ when Ocaml_version.(ocaml_v < Releases.v4_12_0) ->
Ok (String.equal v "1.4.1")
| _ when Ocaml_version.(ocaml_v < Releases.v4_13_0) ->
Ok (String.equal v "1.9.0")
| _ when Ocaml_version.(ocaml_v < Releases.v4_14_0) ->
Ok (String.equal v "1.9.1")
| _ -> Error (`Ocaml_version_not_supported ocaml_v))
| Error e -> Error (`Unexpected e)
| None -> (
match available_versions ocaml_v with
| Some v -> Error (`Newer_available (None, Array.last v))
| None -> Ok ())
| Some v -> (
match available_versions ocaml_v with
| None -> Ok ()
| Some available -> (
match Array.findi available ~f:(fun _ -> String.equal v) with
| None -> Ok ()
| Some (pos, _) ->
let last = Array.length available - 1 in
if Int.equal pos last then Ok ()
else Error (`Newer_available (Some v, available.(last)))))
in
Result.map_error res ~f:(fun err ->
let msg =
match err with
| `Ocaml_version_not_supported v ->
| `Newer_available (old, new_) ->
let upgrade =
match old with
| None -> sprintf "to %s" new_
| Some old -> sprintf "%s to %s" old new_
in
sprintf
"The installed version of OCaml is not supported by `ocamllsp`: \
%s. Consider upgrading OCaml version used in the current sandbox."
(Ocaml_version.to_string v)
| `Unexpected `Language_server_isn't_ocamllsp ->
"Using a language server besides `ocamllsp` isn't expected by this \
extension. Please, switch to using `ocamllsp` by installing the \
package `ocaml-lsp-server` in your current sandbox."
| `Unexpected `Missing_serverInfo
| `Unexpected `ServerInfo_version_missing ->
"The extension expected the server version to be sent by `ocamllsp`. \
It is missing. Please, consider upgrading the package \
`ocaml-lsp-server`."
| `Unexpected `Unable_to_parse_version ->
"The extension was unable to parse `ocamllsp` version. That's \
strange. Consider filing an issue on the project GitHub with the \
version of your `ocamllsp`."
"The is a newer version of ocamllsp available. Consider upgrading \
%s"
upgrade
in
`Msg msg)

Expand Down
2 changes: 1 addition & 1 deletion src/ocaml_lsp.mli
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ type t
val of_initialize_result : LanguageClient.InitializeResult.t -> t

val is_version_up_to_date :
t -> Ocaml_version.t -> (bool, [ `Msg of string ]) result
t -> Ocaml_version.t -> (unit, [ `Msg of string ]) result

val can_handle_switch_impl_intf : t -> bool

Expand Down

0 comments on commit bcaf28a

Please sign in to comment.