Fix type enclosing deduplication #1864
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deduplication can only be done if the type have been printed. We perform that printing only at the junction between the reconstructed identifier enclosings and the ones from the tree nodes because we often want to keep both. All other duplicated ranges are removed.
voodoos
force-pushed
the
fix-type-enclosing-deduplication
branch
from
5444aa2
to
d6ba564
Compare
xvw
reviewed
Nov 21, 2024
| "line": 1, | ||
| "col": 12 | ||
| }, | ||
| "type": "(module Stdlib__List)", |
There was a problem hiding this comment.
There is a function that replace M__N by M.N, should we use it?
There was a problem hiding this comment.
In fact, that's the work of short-path. If I enable it in the concerned tests the results are the ones we expect. (And maybe we should simply do that for type search too...)
Since deduplciation has been introduced a while ago the results of type-enclosing (the number of enclosings) have been unstable. This cannot be solved easily on the server-side without printing more types which can lead to performance issues when large modules are involed. We now leave the responsability of deduplication to the clients.
|
Deduplication of resulting enclosings have made the type-enclosing query unstable for a while: the number of resulting enclosing could vary depending on the selected index. This PR initially tried to improve deduplication by printing more types, but that can lead to performance issues when these types involve large modules. Doing deduplication while ensuring a minimal performance impact makes the implementation awkward and complex. We finally decided to remove-it all together: clients should filter redundant results as they appear. |
voodoos
added a commit
to voodoos/merlin
that referenced
this pull request
Nov 26, 2024
from voodoos/fix-type-enclosing-deduplication Change entry for ocaml#1854
voodoos
added a commit
to voodoos/opam-repository
that referenced
this pull request
Nov 26, 2024
CHANGES:
Tue Nov 26 17:30:42 CET 2024
+ merlin binary
- Respect the `EXCLUDE_QUERY_DIR` configuration directive when looking for cmt
files (ocaml/merlin#1854)
- Fix occurrences bug in which relative paths in index files are resolved against the
PWD rather than the SOURCE_ROOT (ocaml/merlin#1855)
- Fix exception in polarity search (ocaml/merlin#1858 fixes ocaml/merlin#1113)
- Fix jump to `fun` targets not working (ocaml/merlin#1863, fixes ocaml/merlin#1862)
- Fix type-enclosing results instability. This reverts some overly
aggressive deduplication that should be done on the client side. (ocaml/merlin#1864)
- Fix occurrences not working when the definition comes from a hidden source
file (ocaml/merlin#1865)
voodoos
added a commit
to voodoos/opam-repository
that referenced
this pull request
Nov 26, 2024
CHANGES:
Tue Nov 26 17:30:42 CET 2024
+ merlin binary
- Respect the `EXCLUDE_QUERY_DIR` configuration directive when looking for
cmt files (ocaml/merlin#1854)
- Fix exception in polarity search (ocaml/merlin#1858 fixes ocaml/merlin#1113)
- Fix type-enclosing results instability. This reverts some overly
aggressive deduplication that should be done on the client side. (ocaml/merlin#1864)
voodoos
added a commit
to voodoos/opam-repository
that referenced
this pull request
Nov 26, 2024
CHANGES:
Tue Nov 26 17:30:42 CET 2024
+ merlin binary
- Respect the `EXCLUDE_QUERY_DIR` configuration directive when looking for cmt
files (ocaml/merlin#1854)
- Fix occurrences bug in which relative paths in index files are resolved against the
PWD rather than the SOURCE_ROOT (ocaml/merlin#1855)
- Fix exception in polarity search (ocaml/merlin#1858 fixes ocaml/merlin#1113)
- Fix jump to `fun` targets not working (ocaml/merlin#1863, fixes ocaml/merlin#1862)
- Fix type-enclosing results instability. This reverts some overly
aggressive deduplication that should be done on the client side. (ocaml/merlin#1864)
- Fix occurrences not working when the definition comes from a hidden source
file (ocaml/merlin#1865)
voodoos
added a commit
to voodoos/opam-repository
that referenced
this pull request
Nov 26, 2024
CHANGES:
Tue Nov 26 17:30:42 CET 2024
+ merlin binary
- Respect the `EXCLUDE_QUERY_DIR` configuration directive when looking for
cmt files (ocaml/merlin#1854)
- Fix exception in polarity search (ocaml/merlin#1858 fixes ocaml/merlin#1113)
- Fix type-enclosing results instability. This reverts some overly
aggressive deduplication that should be done on the client side. (ocaml/merlin#1864)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Deduplication can only be done if the type have been printed which is an expensive process. We perform that printing only at the junction between the reconstructed identifier's enclosings and the ones from the tree nodes because we often want to keep both (often this gives the generic type and then its specialized version). All other duplicated ranges are removed.