Skip to content

Shorten Merlin diagnostics error - #1513

Merged
voodoos merged 7 commits into
ocaml:masterfrom
xvw:add-shorten-merlin-errors-configuration
Apr 16, 2025
Merged

Shorten Merlin diagnostics error#1513
voodoos merged 7 commits into
ocaml:masterfrom
xvw:add-shorten-merlin-errors-configuration

Conversation

@xvw

@xvw xvw commented Apr 8, 2025

Copy link
Copy Markdown
Member

Make error diagnostics shortenable (to improve readability)

@xvw
xvw requested a review from voodoos April 8, 2025 20:10
@coveralls

coveralls commented Apr 8, 2025 •
edited
Loading

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 4844

Details

  • 62 of 94 (65.96%) changed or added relevant lines in 5 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.2%) to 23.359%
Changes Missing Coverage Covered Lines Changed/Added Lines %
ocaml-lsp-server/src/diagnostics.ml 35 49 71.43%
ocaml-lsp-server/src/config_data.ml 18 36 50.0%
Files with Coverage Reduction New Missed Lines %
ocaml-lsp-server/src/range.ml 1 57.69%
Totals Coverage Status
Change from base Build 4841: 0.2%
Covered Lines: 6002
Relevant Lines: 25695
💛 - Coveralls

voodoos
voodoos approved these changes Apr 8, 2025
View reviewed changes

@voodoos voodoos left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you make the PR or commit messages more descriptive ?

As far as I understand there are multiple things happening here:

  • Optionally limit multi-line errors to only the first line
  • Only show configuration failures when there are some (not optional)
  • Only show typer diagnostics if there are no syntax errors (not optional)

The first one is uncontroversial.

These last two filters could arguably be made more precise, and there used to be a mechanism in Merlin not to report multiple errors related to the same missing module for example.

Also I wonder whether it would make sense to still show typer errors when they are located before the first parser error. But I don't mind keeping it like this for now and see how it feels in practice.

(cc @awilliambauer)

Comment thread ocaml-lsp-server/src/diagnostics.ml Outdated
Range.create ~start ~end_)
;;

let default_error_to_diagnostics ~diagnostics ~merlin ~error =

@voodoos voodoos Apr 8, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't understand that name ? Why is it "default" ?

Comment thread ocaml-lsp-server/src/diagnostics.ml
Comment thread ocaml-lsp-server/src/diagnostics.ml Outdated
@xvw xvw changed the title Shorten Merlin diagnostics error Draft: Shorten Merlin diagnostics error Apr 16, 2025
@xvw xvw changed the title Draft: Shorten Merlin diagnostics error Shorten Merlin diagnostics error Apr 16, 2025
voodoos
voodoos approved these changes Apr 16, 2025
View reviewed changes

@voodoos voodoos left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks Xavier. For the record, in the latest version we only kept the part of that PR adding a new configuration option that only return the first line of multi-line diagnostics.

@voodoos
voodoos merged commit 6d84c1c into ocaml:master Apr 16, 2025
@xvw
xvw deleted the add-shorten-merlin-errors-configuration branch April 16, 2025 21:05
voodoos pushed a commit to voodoos/ocaml-lsp that referenced this pull request Jun 6, 2025
@xvw @voodoos
Shorten Merlin diagnostics error (ocaml#1513)
5dc99a7 
* Add ShortenMerlinDiagnostics in `config_data` (and promote)

* Upgrade configuration

* Change diagnostics representation

* Add test

* Remove useless `Fiber.return`

* Add CHANGES entry

* Restore Merlin's recovery
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 23, 2025
@voodoos
[new release] lsp (3 packages) (1.21.0-4.14)
6ef1db7 
CHANGES:

## Features

- Make `inlay-hint` for function parameters configurable (ocaml/ocaml-lsp#1515)

- Add custom `ocamllsp/jumpToTypedHole` to navigate through typed holes (ocaml/ocaml-lsp#1516)

- Add a code-action for combining pattern cases (just relaying on regex) (ocaml/ocaml-lsp#1514)

- Allow (by configuration) shortening of diagnostics (just highlighting the first line) (ocaml/ocaml-lsp#1513)

- Fix `yojson_of_t` for `Nullable_option`: serialize `None` as `Null` instead of asserting false (ocaml/ocaml-lsp#1525 fixes ocaml/ocaml-lsp#1524)

## Fixes

- Support for `class`, `class type`, `method` and `property` for `DocumentSymbol` query (ocaml/ocaml-lsp#1487 fixes ocaml/ocaml-lsp#1449)

- Fix `inlay-hint` for function parameters (ocaml/ocaml-lsp#1515)

- More precise diagnostics in the event of a failed identifier search (`Definition_query`) (ocaml/ocaml-lsp#1518)

- Remove `ocamlformat` application after `destruct` (that remove some useful parenthesis) (ocaml/ocaml-lsp#1519)

- Add a new server option `standardHover`, that can be used by clients to
  disable the default hover provider.  When `standardHover = false`
  `textDocument/hover` requests always returns with empty result. (ocaml/ocaml-lsp#1416)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 23, 2025
@voodoos
[new release] lsp (3 packages) (1.21.0-4.14)
edc6339 
CHANGES:

## Features

- Make `inlay-hint` for function parameters configurable (ocaml/ocaml-lsp#1515)

- Add custom `ocamllsp/jumpToTypedHole` to navigate through typed holes (ocaml/ocaml-lsp#1516)

- Add a code-action for combining pattern cases (just relaying on regex) (ocaml/ocaml-lsp#1514)

- Allow (by configuration) shortening of diagnostics (just highlighting the first line) (ocaml/ocaml-lsp#1513)

- Fix `yojson_of_t` for `Nullable_option`: serialize `None` as `Null` instead of asserting false (ocaml/ocaml-lsp#1525 fixes ocaml/ocaml-lsp#1524)

## Fixes

- Support for `class`, `class type`, `method` and `property` for `DocumentSymbol` query (ocaml/ocaml-lsp#1487 fixes ocaml/ocaml-lsp#1449)

- Fix `inlay-hint` for function parameters (ocaml/ocaml-lsp#1515)

- More precise diagnostics in the event of a failed identifier search (`Definition_query`) (ocaml/ocaml-lsp#1518)

- Remove `ocamlformat` application after `destruct` (that remove some useful parenthesis) (ocaml/ocaml-lsp#1519)

- Add a new server option `standardHover`, that can be used by clients to
  disable the default hover provider.  When `standardHover = false`
  `textDocument/hover` requests always returns with empty result. (ocaml/ocaml-lsp#1416)
@voodoos voodoos mentioned this pull request Jun 23, 2025
Merged
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 24, 2025
@voodoos
[new release] lsp (3 packages) (1.23.0)
4303a86 
CHANGES:

## Features

- Make `inlay-hint` for function parameters configurable (ocaml/ocaml-lsp#1515)
- Add custom `ocamllsp/jumpToTypedHole` to navigate through typed holes (ocaml/ocaml-lsp#1516)
- Add a code-action for combining pattern cases (just relaying on regex) (ocaml/ocaml-lsp#1514)
- Allow (by configuration) shortening of diagnostics (just highlighting the first line) (ocaml/ocaml-lsp#1513)
- Fix `yojson_of_t` for `Nullable_option`: serialize `None` as `Null` instead of asserting false (ocaml/ocaml-lsp#1525 fixes ocaml/ocaml-lsp#1524)

## Fixes

- Support for `class`, `class type`, `method` and `property` for `DocumentSymbol` query (ocaml/ocaml-lsp#1487 fixes ocaml/ocaml-lsp#1449)
- Fix `inlay-hint` for function parameters (ocaml/ocaml-lsp#1515)
- More precise diagnostics in the event of a failed identifier search (`Definition_query`) (ocaml/ocaml-lsp#1518)
- Remove `ocamlformat` application after `destruct` (that remove some useful parenthesis) (ocaml/ocaml-lsp#1519)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Jun 24, 2025
@voodoos
[new release] lsp (3 packages) (1.23.0)
f9607eb 
CHANGES:

## Features

- Make `inlay-hint` for function parameters configurable (ocaml/ocaml-lsp#1515)
- Add custom `ocamllsp/jumpToTypedHole` to navigate through typed holes (ocaml/ocaml-lsp#1516)
- Add a code-action for combining pattern cases (just relaying on regex) (ocaml/ocaml-lsp#1514)
- Allow (by configuration) shortening of diagnostics (just highlighting the first line) (ocaml/ocaml-lsp#1513)
- Fix `yojson_of_t` for `Nullable_option`: serialize `None` as `Null` instead of asserting false (ocaml/ocaml-lsp#1525 fixes ocaml/ocaml-lsp#1524)

## Fixes

- Support for `class`, `class type`, `method` and `property` for `DocumentSymbol` query (ocaml/ocaml-lsp#1487 fixes ocaml/ocaml-lsp#1449)
- Fix `inlay-hint` for function parameters (ocaml/ocaml-lsp#1515)
- More precise diagnostics in the event of a failed identifier search (`Definition_query`) (ocaml/ocaml-lsp#1518)
- Remove `ocamlformat` application after `destruct` (that remove some useful parenthesis) (ocaml/ocaml-lsp#1519)
@voodoos voodoos mentioned this pull request Jun 24, 2025
Merged
davesnx pushed a commit to davesnx/ocaml-lsp that referenced this pull request Nov 26, 2025
@xvw @davesnx
Shorten Merlin diagnostics error (ocaml#1513)
3838e2c 
* Add ShortenMerlinDiagnostics in `config_data` (and promote)

* Upgrade configuration

* Change diagnostics representation

* Add test

* Remove useless `Fiber.return`

* Add CHANGES entry

* Restore Merlin's recovery
@davesnx davesnx mentioned this pull request Nov 26, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@voodoos voodoos voodoos approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xvw @coveralls @voodoos