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

Improve stale occurrences #123

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Improve stale occurrences #123

wants to merge 3 commits into from

Conversation

liam923
Copy link
Contributor

@liam923 liam923 commented Jan 8, 2025

When Merlin finds project-wide occurrences, it loads lists of occurrences from index files. Those index files contain metadata about the state of the ml/mli files when the index was generated. This is used as a heuristic to determine if occurrences might be stale (ie, that occurrence was either moved or removed since the index file was generated).

This PR does two things:

  1. Fix a bug with Merlin's detection of stale occurrences. Currently, depending on the layout of a repository, Merlin may not line up file paths correctly.
  2. Currently, Merlin discards any occurrences it thinks might be stale. This PR changes this so that the stale occurrences are reported, but flagged as being stale.

The net effect for an end user should be nothing - the stale occurrences should still be reported by their editor. But this enables future LSP/editor work.

@liam923 liam923 requested a review from goldfirere January 8, 2025 16:17
@goldfirere
Copy link
Contributor

As discussed in person, @liam923 will explore the possibility of getting a review from Ulysse, who is more expert in this code. If there is somehow trouble, please call me back into service here.

@goldfirere goldfirere removed their request for review January 9, 2025 18:32
@liam923
Copy link
Contributor Author

liam923 commented Jan 10, 2025

I've opened a PR upstream: ocaml/merlin#1885

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.

2 participants