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

refactor: get doc kind info from merlin #1237

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

andreypopp
Copy link
Member

Currently doc kind (impl vs intf) is hardcoded as a set of file extensions.

Instead this PR tried to get kind info from merlin first, and if it fails, fallback to the old way.

Ref: ocaml/dune#8567

@coveralls
Copy link

coveralls commented Apr 1, 2024

Pull Request Test Coverage Report for Build 4121

Details

  • 19 of 42 (45.24%) changed or added relevant lines in 6 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.02%) to 21.048%

Changes Missing Coverage Covered Lines Changed/Added Lines %
ocaml-lsp-server/src/code_actions/action_open_related.ml 3 4 75.0%
ocaml-lsp-server/src/ocaml_lsp_server.ml 0 1 0.0%
ocaml-lsp-server/src/custom_requests/req_switch_impl_intf.ml 0 8 0.0%
ocaml-lsp-server/src/document.ml 13 26 50.0%
Files with Coverage Reduction New Missed Lines %
ocaml-lsp-server/src/document.ml 1 59.02%
ocaml-lsp-server/src/custom_requests/req_switch_impl_intf.ml 2 0.0%
Totals Coverage Status
Change from base Build 4108: 0.02%
Covered Lines: 5039
Relevant Lines: 23941

💛 - Coveralls

@@ -97,7 +97,7 @@ val close : t -> unit Fiber.t
counterparts for the URI [uri].

For instance, the counterparts of the file [/file.ml] are [/file.mli]. *)
val get_impl_intf_counterparts : Uri.t -> Uri.t list
val get_impl_intf_counterparts : Merlin.t option -> Uri.t -> Uri.t list
Copy link
Member Author

Choose a reason for hiding this comment

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

I kept this to accept Merlin.t option as I understand it still could work for docs outside of the current LSP workspace (e.g. do jump to definition to some opam installed lib and then still can use this switch impl/intf command to switch between .ml/.mli).

Copy link
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

LGTM. Let's see what @voodoos says about using the merlin API this way

@voodoos
Copy link
Collaborator

voodoos commented Apr 3, 2024

Looking at the configuration suffixes before fallbacking to standard extensions seems like the correct thing to do, yes.

@rgrinberg
Copy link
Member

Could you add a CAHNGES entry? Maybe include a test as well. I have no idea how hard it is to write one for this feature though.

@andreypopp
Copy link
Member Author

Updated CHANGES under Fixes. But not clear to me, indeed, how to test this easily...

@rgrinberg rgrinberg merged commit cb338c7 into ocaml:master Apr 4, 2024
9 checks passed
@andreypopp andreypopp deleted the merlin-suffix branch April 25, 2024 18:18
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jul 5, 2024
CHANGES:

## Features

- Introduce a configuration option to control dune diagnostics. The option is
  called `duneDiganostics` and it may be set to `{ enable: false }` to disable
  diagnostics. (ocaml/ocaml-lsp#1221)

- Support folding of `ifthenelse` expressions (ocaml/ocaml-lsp#1031)

- Improve hover behavior (ocaml/ocaml-lsp#1245)

  Hovers are no longer displaye on useless parsetree nodes such as keywords,
  comments, etc.

  Multiline hovers are now filtered away.

  Display expanded ppx's in the hover window.

- Improve document symbols (ocaml/ocaml-lsp#1247)

  Use the parse tree instead of the typed tree. This means that document
  symbols will work even if the source code doesn't type check.

  Include symbols at arbitrary depth.

  Differentiate functions / types / variants / etc.

  This now includes PPXs like `let%expect_test` or `let%bench` in the outline.

- Introduce a `destruct-line` code action. This is an improved version of the
  old `destruct` code action. (ocaml/ocaml-lsp#1283)

- Improve signature inference to only include types for elements that were
  absent from the signature. Previously, all signature items would always be
  inserted. (ocaml/ocaml-lsp#1289)

- Add an `update-signature` code action to update the types of elements that
  were already present in the signature (ocaml/ocaml-lsp#1289)

- Add custom
  [`ocamllsp/merlinCallCompatible`](https://github.com/ocaml/ocaml-lsp/blob/e165f6a3962c356adc7364b9ca71788e93489dd0/ocaml-lsp-server/docs/ocamllsp/merlinCallCompatible-spec.md)
  request (ocaml/ocaml-lsp#1265)

- Add custom [`ocamllsp/typeEnclosing`](https://github.com/ocaml/ocaml-lsp/blob/109801e56f2060caf4487427bede28b824f4f1fe/ocaml-lsp-server/docs/ocamllsp/typeEnclosing-spec.md) request (ocaml/ocaml-lsp#1304)

## Fixes

- Detect document kind by looking at merlin's `suffixes` config.

  This enables more lsp features for non-.ml/.mli files. Though it still
  depends on merlin's support. (ocaml/ocaml-lsp#1237)

- Correctly accept the `--clientProcessId` flag. (ocaml/ocaml-lsp#1242)

- Disable automatic completion and signature help inside comments (ocaml/ocaml-lsp#1246)

- Includes a new optional/configurable option to toggle syntax documentation. If
  toggled on, allows display of syntax documentation on hover tooltips. Can be
  controlled via environment variables and by GUI for VS code. (ocaml/ocaml-lsp#1218)

- For completions on labels that the LSP gets from merlin, take into account
  whether the prefix being completed starts with `~` or `?`. Change the label
  completions that start with `?` to start with `~` when the prefix being
  completed starts with `~`. (ocaml/ocaml-lsp#1277)

- Fix document syncing (ocaml/ocaml-lsp#1278, ocaml/ocaml-lsp#1280, fixes ocaml/ocaml-lsp#1207)

- Stop generating inlay hints on generated code (ocaml/ocaml-lsp#1290)

- Fix parenthesizing of function types in `SignatureHelp` (ocaml/ocaml-lsp#1296)

- Fix syntax documentation rendering (ocaml/ocaml-lsp#1318)
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
CHANGES:

## Features

- Introduce a configuration option to control dune diagnostics. The option is
  called `duneDiganostics` and it may be set to `{ enable: false }` to disable
  diagnostics. (ocaml/ocaml-lsp#1221)

- Support folding of `ifthenelse` expressions (ocaml/ocaml-lsp#1031)

- Improve hover behavior (ocaml/ocaml-lsp#1245)

  Hovers are no longer displaye on useless parsetree nodes such as keywords,
  comments, etc.

  Multiline hovers are now filtered away.

  Display expanded ppx's in the hover window.

- Improve document symbols (ocaml/ocaml-lsp#1247)

  Use the parse tree instead of the typed tree. This means that document
  symbols will work even if the source code doesn't type check.

  Include symbols at arbitrary depth.

  Differentiate functions / types / variants / etc.

  This now includes PPXs like `let%expect_test` or `let%bench` in the outline.

- Introduce a `destruct-line` code action. This is an improved version of the
  old `destruct` code action. (ocaml/ocaml-lsp#1283)

- Improve signature inference to only include types for elements that were
  absent from the signature. Previously, all signature items would always be
  inserted. (ocaml/ocaml-lsp#1289)

- Add an `update-signature` code action to update the types of elements that
  were already present in the signature (ocaml/ocaml-lsp#1289)

- Add custom
  [`ocamllsp/merlinCallCompatible`](https://github.com/ocaml/ocaml-lsp/blob/e165f6a3962c356adc7364b9ca71788e93489dd0/ocaml-lsp-server/docs/ocamllsp/merlinCallCompatible-spec.md)
  request (ocaml/ocaml-lsp#1265)

- Add custom [`ocamllsp/typeEnclosing`](https://github.com/ocaml/ocaml-lsp/blob/109801e56f2060caf4487427bede28b824f4f1fe/ocaml-lsp-server/docs/ocamllsp/typeEnclosing-spec.md) request (ocaml/ocaml-lsp#1304)

## Fixes

- Detect document kind by looking at merlin's `suffixes` config.

  This enables more lsp features for non-.ml/.mli files. Though it still
  depends on merlin's support. (ocaml/ocaml-lsp#1237)

- Correctly accept the `--clientProcessId` flag. (ocaml/ocaml-lsp#1242)

- Disable automatic completion and signature help inside comments (ocaml/ocaml-lsp#1246)

- Includes a new optional/configurable option to toggle syntax documentation. If
  toggled on, allows display of syntax documentation on hover tooltips. Can be
  controlled via environment variables and by GUI for VS code. (ocaml/ocaml-lsp#1218)

- For completions on labels that the LSP gets from merlin, take into account
  whether the prefix being completed starts with `~` or `?`. Change the label
  completions that start with `?` to start with `~` when the prefix being
  completed starts with `~`. (ocaml/ocaml-lsp#1277)

- Fix document syncing (ocaml/ocaml-lsp#1278, ocaml/ocaml-lsp#1280, fixes ocaml/ocaml-lsp#1207)

- Stop generating inlay hints on generated code (ocaml/ocaml-lsp#1290)

- Fix parenthesizing of function types in `SignatureHelp` (ocaml/ocaml-lsp#1296)

- Fix syntax documentation rendering (ocaml/ocaml-lsp#1318)
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.

4 participants