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

[new release] ocaml-lsp-server, lsp and jsonrpc (1.16.0-4.14) #23950

Closed
wants to merge 2 commits into from

Conversation

rgrinberg
Copy link
Member

LSP Server for OCaml

CHANGES:

Fixes

Features

CHANGES:

## Fixes

- Disable code lens by default. The support can be re-enabled by explicitly
  setting it in the configuration. (ocaml/ocaml-lsp#1134)

- Fix initilization of `ocamlformat-rpc` in some edge cases when ocamlformat is
  initialized concurrently (ocaml/ocaml-lsp#1132)

- Kill unnecessary `$ dune ocaml-merlin` with SIGTERM rather than SIGKILL
  (ocaml/ocaml-lsp#1124)

- Refactor comment parsing to use `odoc-parser` and `cmarkit` instead of
  `octavius` and `omd` (ocaml/ocaml-lsp#1088)

  This allows users who migrated to omd 2.X to install ocaml-lsp-server in the
  same opam switch.

  We also slightly improved markdown generation support and fixed a couple in
  the generation of inline heading and module types.

- Allow opening documents that were already open. This is a workaround for
  neovim's lsp client (ocaml/ocaml-lsp#1067)

- Disable type annotation for functions (ocaml/ocaml-lsp#1054)

- Respect codeActionLiteralSupport capability (ocaml/ocaml-lsp#1046)

- Fix a document syncing issue when utf-16 is the position encoding (ocaml/ocaml-lsp#1004)

- Disable "Type-annotate" action for code that is already annotated.
  ([ocaml/ocaml-lsp#1037](ocaml/ocaml-lsp#1037)), fixes
  [ocaml/ocaml-lsp#1036](ocaml/ocaml-lsp#1036)

- Fix semantic highlighting of long identifiers when using preprocessors
  ([ocaml/ocaml-lsp#1049](ocaml/ocaml-lsp#1049), fixes
  [ocaml/ocaml-lsp#1034](ocaml/ocaml-lsp#1034))

- Fix the type of DocumentSelector in cram document registration (ocaml/ocaml-lsp#1068)

- Accept the `--clientProcessId` command line argument. (ocaml/ocaml-lsp#1074)

- Accept `--port` as a synonym for `--socket`. (ocaml/ocaml-lsp#1075)

- Fix connecting to dune rpc on Windows. (ocaml/ocaml-lsp#1080)

## Features

- Add "Remove type annotation" code action. (ocaml/ocaml-lsp#1039)

- Support settings through `didChangeConfiguration` notification (ocaml/ocaml-lsp#1103)

- Add "Extract local" and "Extract function" code actions. (ocaml/ocaml-lsp#870)

- Depend directly on `merlin-lib` 4.9 (ocaml/ocaml-lsp#1070)
@tmattio
Copy link
Contributor

tmattio commented Jun 18, 2023

Now that ocaml-lsp-server uses merlin as a library, I think we can remove the compiler version from the package version (use 1.16.0 instead of 1.16.0-4.14)

@rgrinberg
Copy link
Member Author

Now that ocaml-lsp-server uses merlin as a library, I think we can remove the compiler version from the package version (use 1.16.0 instead of 1.16.0-4.14)

I'll need to test this. It's highly unlikely though as we rely on the parsetree and typedtree directly in some places.

@voodoos
Copy link
Contributor

voodoos commented Jun 19, 2023

Ah yes of course. I can also do the fixes if you want.

At some point we should upstream the features that relies on the Typedtree.

@voodoos
Copy link
Contributor

voodoos commented Jun 19, 2023

Now that ocaml-lsp-server uses merlin as a library, I think we can remove the compiler version from the package version (use 1.16.0 instead of 1.16.0-4.14)

I'll need to test this. It's highly unlikely though as we rely on the parsetree and typedtree directly in some places.

I think it should work with OCaml 5.0 since no changes were made to the ASTs. I tested the build on my machine and it looks ok. So you can probably relax the constraint a bit.

5.1 has a few changes that might be possible to handle in a generic way (like catch-all cases).

@voodoos
Copy link
Contributor

voodoos commented Jun 19, 2023

Also the patch to ensure compatibility with 5.1 without breaking 4.14/5.0 build is very constraint:

diff --git a/ocaml-lsp-server/src/folding_range.ml b/ocaml-lsp-server/src/folding_range.ml
index 7b132ebf..c4f87d70 100644
--- a/ocaml-lsp-server/src/folding_range.ml
+++ b/ocaml-lsp-server/src/folding_range.ml
@@ -72,7 +72,7 @@ let fold_over_parsetree (parsetree : Mreader.parsetree) =
       | Parsetree.Pmod_ident _
       | Parsetree.Pmod_apply (_, _)
       | Parsetree.Pmod_constraint (_, _)
-      | Parsetree.Pmod_unpack _ | Parsetree.Pmod_extension _ ->
+      | Parsetree.Pmod_unpack _ | Parsetree.Pmod_extension _ |  (*Pmod_apply_unit*) _ ->
         Ast_iterator.default_iterator.module_expr self module_expr
     in
 
diff --git a/ocaml-lsp-server/src/semantic_highlighting.ml b/ocaml-lsp-server/src/semantic_highlighting.ml
index 01158b44..94b19f25 100644
--- a/ocaml-lsp-server/src/semantic_highlighting.ml
+++ b/ocaml-lsp-server/src/semantic_highlighting.ml
@@ -421,6 +421,7 @@ end = struct
     self.typ self pld_type;
     self.attributes self pld_attributes
 
+  [@@@ocaml.warning "-9"]
   let value_binding (self : Ast_iterator.iterator)
       ({ pvb_pat; pvb_expr; pvb_attributes; pvb_loc = _ } as vb :
         Parsetree.value_binding) =
@@ -749,7 +750,7 @@ end = struct
           self.module_type self mt);
         `Custom_iterator
       | Pmod_extension _ -> `Custom_iterator
-      | Pmod_unpack _ | Pmod_apply (_, _) | Pmod_structure _ ->
+      | Pmod_unpack _ | Pmod_apply (_, _) | Pmod_structure _ |  (*Pmod_apply_unit*) _ ->
         `Default_iterator
     with
     | `Custom_iterator -> self.attributes self pmod_attributes

@rgrinberg It might be worth-it to re-release with these changes so that this release will actually be compatible with all latest 3 versions of OCaml ?

@rgrinberg
Copy link
Member Author

We don't "re-release" anything but we can bump the patch version.

@tmattio
Copy link
Contributor

tmattio commented Jun 19, 2023

We don't "re-release" anything but we can bump the patch version.

I think what @voodoos was suggesting is that since this PR hasn't been merged, we could close it and open a new one that contains the patch above, to avoid adding versions to the opam-repository we don't want users to use.

@rgrinberg
Copy link
Member Author

Okay, we can close this PR after I create a new one.

@mseri
Copy link
Member

mseri commented Jun 20, 2023

Does #23969 supersede this PR?

@voodoos
Copy link
Contributor

voodoos commented Jun 21, 2023

Does #23969 supersede this PR?

Yes it does, this one can be closed.

@rgrinberg rgrinberg closed this Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants