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

wip: compatibility with merlin-lib 5.1-502 #1233

Merged
merged 22 commits into from
Jul 30, 2024
Merged

Conversation

voodoos
Copy link
Collaborator

@voodoos voodoos commented Feb 23, 2024

Preliminary support for the 414-5.2~preview version of merlin-lib for ocaml 5.2.

@jfeser the changes in the representation of functions breaks the beta-reduction feature of the action-inline code action. It would be great if you could find some time to restore it before the final release (sometime in April I think).

@jfeser
Copy link
Collaborator

jfeser commented Feb 27, 2024

Dumb question, but how do I get the right version of merlin for building this PR? I've been using nix and I'm not very familiar with the opam pinning workflow.

@voodoos
Copy link
Collaborator Author

voodoos commented Feb 28, 2024

I cannot help you with the nix setup, but there is a preview version of Merlin released on opam that should work with that pr: 4.14-502~preview

@jfeser
Copy link
Collaborator

jfeser commented May 3, 2024

I've pushed some commits here: https://github.com/jfeser/ocaml-lsp/tree/merlin-update. The new version of beta reduction is more correct but less aggressive. It only reduces fully applied functions and does not attempt to reduce functions with labeled parameters.

@voodoos
Copy link
Collaborator Author

voodoos commented May 6, 2024

Thanks for taking the time @jfeser !
I will cherry-pick these commits to the update's branch.

.ocamlformat Outdated Show resolved Hide resolved
@rgrinberg
Copy link
Member

Dumb question, but how do I get the right version of merlin for building this PR? I've been using nix and I'm not very familiar with the opam pinning workflow.

I've added a flake input for merlin here #1264. So you can just set the branch/tag/commit in the flake.nix and then run nix develop.

@voodoos
Copy link
Collaborator Author

voodoos commented May 22, 2024

@rgrinberg what are your plans for 5.2 support ? (Merlin 5.0-502 with which that PR is compatible was just released.)

With the non-trivial typedtree changes in 5.2, the update breaks the multi-version support we managed to kept for some time.
Additionally, we cannot get any of the CI running without bumping ocamlformat since they all try to install it...

Should we aim at merging this PR (there is some work to do on the CI still) and then cut a release ?
Or do you prefer that I open a branch for 5.2 and release from that branch ?

@rgrinberg
Copy link
Member

My plan was to wait for @awilliambauer to finish submitting his series of patches and make a release. So that we can achieve the following two goals:

  1. Release all the new features to users of 4.14 and 5.1
  2. Sync the fork of ocamllsp and upstream

Once that is done, we'd no longer need to care about preserving 5.1 (and below) compatibility in the master branch

@awilliambauer
Copy link
Contributor

Just a heads up there will be some delay getting the rest of the patches submitted. It will be a little while before I have more time to look into the issue with #1290. We also have a patch to add more inlay hint configuration that involves changes to typedtree stuff that is different upstream, and it will take some work to port and test that.

@rgrinberg
Copy link
Member

rgrinberg commented May 28, 2024 via email

@voodoos voodoos changed the title wip: compatibility with merlin-lib 414-5.2~preview wip: compatibility with merlin-lib 5.1-502 Jun 10, 2024
@voodoos
Copy link
Collaborator Author

voodoos commented Jun 10, 2024

@rgrinberg there is no ppx_expect.common library since 0.17.0 I updated the dune files in 2aee922.

@voodoos voodoos force-pushed the 5.2-preview branch 2 times, most recently from a644bff to e9ed7d2 Compare June 11, 2024 08:42
@rgrinberg
Copy link
Member

@rgrinberg there is no ppx_expect.common library since 0.17.0 I updated the dune files in 2aee922.

Can this be done in a separate PR?

@voodoos
Copy link
Collaborator Author

voodoos commented Jun 11, 2024

Can this be done in a separate PR?

Will do.

About the release plan, what I have in mind right now is:

@rgrinberg rgrinberg force-pushed the 5.2-preview branch 2 times, most recently from ce036f2 to 3ac8ec6 Compare June 14, 2024 02:14
@anmonteiro
Copy link
Contributor

This exception occurs when I try to hover on a module with inconsistent assumptions over interface.

you may want to run esy / ocaml-lsp inside the build sandbox rather than the execution sandbox (i.e. esy b dune build)

voodoos and others added 16 commits July 26, 2024 13:22
Use merlin-lib config types and utilities

Bump  merlin-lib and ocaml-format versions
Note: function applications will only be eliminated when they are fully applied
and none of the arguments are labeled.
This is due to Typedtree incompatibilities.
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
@coveralls
Copy link

coveralls commented Jul 26, 2024

Pull Request Test Coverage Report for Build 4465

Details

  • 30 of 60 (50.0%) changed or added relevant lines in 11 files are covered.
  • 10 unchanged lines in 7 files lost coverage.
  • Overall coverage decreased (-0.1%) to 21.587%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ocaml-lsp-server/src/code_actions/action_add_rec.ml 0 1 0.0%
ocaml-lsp-server/src/code_actions/action_mark_remove_unused.ml 1 2 50.0%
ocaml-lsp-server/src/document_symbol.ml 0 1 0.0%
ocaml-lsp-server/src/hover_req.ml 0 1 0.0%
ocaml-lsp-server/src/merlin_config.ml 2 3 66.67%
ocaml-lsp-server/src/rename.ml 0 1 0.0%
ocaml-lsp-server/src/workspace_symbol.ml 0 1 0.0%
ocaml-lsp-server/src/code_actions/action_inline.ml 23 26 88.46%
ocaml-lsp-server/src/semantic_highlighting.ml 3 11 27.27%
ocaml-lsp-server/src/ocaml_lsp_server.ml 0 12 0.0%
Files with Coverage Reduction New Missed Lines %
ocaml-lsp-server/src/ocaml_lsp_server.ml 1 36.56%
ocaml-lsp-server/src/code_actions/action_type_annotate.ml 1 87.8%
ocaml-lsp-server/src/merlin_config.ml 1 84.96%
ocaml-lsp-server/src/code_actions/action_remove_type_annotation.ml 1 89.19%
ocaml-lsp-server/src/semantic_highlighting.ml 2 51.92%
ocaml-lsp-server/src/hover_req.ml 2 43.23%
ocaml-lsp-server/src/inlay_hints.ml 2 76.06%
Totals Coverage Status
Change from base Build 4456: -0.1%
Covered Lines: 5472
Relevant Lines: 25349

💛 - Coveralls

@voodoos voodoos merged commit 4e74156 into ocaml:master Jul 30, 2024
7 checks passed
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jul 30, 2024
CHANGES:

## Features

- Add custom [`ocamllsp/getDocumentation`](/ocaml-lsp-server/docs/ocamllsp/getDocumentation-spec.md) request (ocaml/ocaml-lsp#1336)
- Add support for OCaml 5.2 (ocaml/ocaml-lsp#1233)

## Fixes

- Kill unnecessary ocamlformat processes with sigterm rather than sigint or
  sigkill (ocaml/ocaml-lsp#1343)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jul 30, 2024
CHANGES:

## Features

- Add custom [`ocamllsp/getDocumentation`](/ocaml-lsp-server/docs/ocamllsp/getDocumentation-spec.md) request (ocaml/ocaml-lsp#1336)
- Add support for OCaml 5.2 (ocaml/ocaml-lsp#1233)

## Fixes

- Kill unnecessary ocamlformat processes with sigterm rather than sigint or
  sigkill (ocaml/ocaml-lsp#1343)
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
CHANGES:

## Features

- Add custom [`ocamllsp/getDocumentation`](/ocaml-lsp-server/docs/ocamllsp/getDocumentation-spec.md) request (ocaml/ocaml-lsp#1336)
- Add support for OCaml 5.2 (ocaml/ocaml-lsp#1233)

## Fixes

- Kill unnecessary ocamlformat processes with sigterm rather than sigint or
  sigkill (ocaml/ocaml-lsp#1343)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants