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.

Sign up for GitHub

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

Jump to bottom

Improving hover behavior #1245

Merged
merged 4 commits into from
May 11, 2024
Merged

Improving hover behavior #1245

merged 4 commits into from
May 11, 2024

Conversation

awilliambauer
Copy link
Contributor

This PR makes several changes to hover behavior to reduce overly noisy hovers, and adds hovering on some PPXes to see the PPX expansion.

Restricting unhelpful hovers

The LSP currently provides hovers in many places that are unhelpful and noisy, including on comments and many keywords.
This patch, instead of providing a hover on every position where Document.Merlin.type_enclosing returns a result, uses the hover_at_cursor function to iterate over the parsetree and select the subset of nodes where hover information is useful.
The primary impact of this is the LSP no longer provides hovers on whitespace, comments, or most keywords and punctuation.

Suppressing multiline highlights

The TextDocumentHighlight request can highlight very large ranges in a way that's distracting and not helpful.
This happens frequently with some PPXes.
This PR adds logic to filter out highlights with multiline ranges.

Adding hovers on PPXes

This PR adds hovers on some PPXes that show the expanded code.

awilliambauer and others added 4 commits May 6, 2024 17:26
@awilliambauer
restricting hovers in various unhelpful contexts, adding hovers on PP…
eead774 
…Xes, and suppressing noisy multiline hovers
@rgrinberg
fmt
477da22 
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
@rgrinberg
_
ec63954 
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
@rgrinberg
_
a8bb6dd 
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
@rgrinberg rgrinberg added this to the 1.18.0 milestone May 11, 2024
@rgrinberg
Copy link
Member

Awesome work. I fixed up the tests, formatting, and added a CHANGES entry.

@rgrinberg rgrinberg merged commit a98fe15 into ocaml:master May 11, 2024
9 checks passed
@PizieDust
Copy link
Contributor

Currently, it doesn't handle:

  • hover over exceptions

It is my understanding that PPX derivers work on 8 different nodes:

  • type declaration in structure
  • type declaration in signature
  • type extension in structure
  • type extension in signature
  • exception in structure
  • exception in signature
  • module type declaration in structure
  • module type declaration in signature

I think this PR handles mostly only type_declarations and won't display the expanded ppx for other types of nodes.

voodoos added a commit to voodoos/opam-repository that referenced this pull request Jul 5, 2024
@voodoos
[new release] lsp (3 packages) (1.18.0)
43adbc2 
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)
@voodoos voodoos mentioned this pull request Jul 5, 2024
Merged
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
@voodoos @avsm
[new release] lsp (3 packages) (1.18.0)
405800d 
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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.18.0
Development

Successfully merging this pull request may close these issues.
3 participants
@awilliambauer @rgrinberg @PizieDust