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: remove intrusive version checking #859

Merged
merged 1 commit into from
Feb 14, 2022
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
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" |])
Comment on lines +67 to +78
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: this could use comments or names, eg ocaml_4_12 = (4, 12)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Btw, ocamlc version library should have values corresponding to ocamlc versions, ie we could use Ocaml_version.v4_12 instead of (4, 12)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ie we could use Ocaml_version.v4_12 instead of (4, 12)

This could work, but is it really clarifying anything? We are mapping version OCaml versions prefixes to ocamllsp versions.

I'll add some comments

]
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