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

Fix EXCLUDE_QUERY_DIR for cmt files #1854

Merged
merged 5 commits into from
Oct 8, 2024
Merged

Fix EXCLUDE_QUERY_DIR for cmt files #1854

merged 5 commits into from
Oct 8, 2024

Conversation

liam923
Copy link
Contributor

@liam923 liam923 commented Oct 7, 2024

When looking for .cmt files, Merlin doesn't currently respect the EXCLUDE_QUERY_DIR directive (see the first commit for a test case demonstrating the issue). This PR resolves this issue.

This PR corresponds to PR 104 on Merlin-JST

@liam923 liam923 mentioned this pull request Oct 7, 2024
Merged
voodoos
voodoos approved these changes Oct 8, 2024
View reviewed changes
Copy link
Collaborator

@voodoos voodoos left a comment

Choose a reason for hiding this comment

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

Thanks Liam the changes look good to me. I wonder whether we whould de-duplicate the result similarly ar what is done for the build_path ?

Could you add a changelog entry and filter the test output so that the path is not printed ?

@liam923
Copy link
Contributor Author

liam923 commented Oct 8, 2024

Thanks Liam the changes look good to me. I wonder whether we whould de-duplicate the result similarly ar what is done for the build_path ?

Good point. Actually at this point build_path and cmt_path are nearly identical, so I pulled their functionality out into a common function.

@voodoos voodoos merged commit bc26853 into ocaml:main Oct 8, 2024
3 checks passed
voodoos added a commit that referenced this pull request Oct 8, 2024
@voodoos
Add changelog entry for #1854
d6068a4
@liam923 liam923 deleted the exclude-query-dir-fix branch October 8, 2024 15:11
voodoos added a commit that referenced this pull request Nov 26, 2024
@voodoos
Change entry for #1854
18fbad6
voodoos added a commit to voodoos/merlin that referenced this pull request Nov 26, 2024
@voodoos
[B] ocaml#1854 Fix EXCLUDE_QUERY_DIR for cmt files
3e61631 
from liam923/exclude-query-dir-fix

Changelog entry for ocaml#1854
voodoos added a commit to voodoos/merlin that referenced this pull request Nov 26, 2024
@voodoos
[B] ocaml#1864 Fix type enclosing deduplication
758f839 
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
@voodoos
[new release] merlin (4 packages) (5.3-502)
252e0d4 
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
@voodoos
[new release] merlin (3 packages) (4.18-414)
5ab8f9b 
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
@voodoos
[new release] merlin (4 packages) (5.3-502)
d5de66f 
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
@voodoos
[new release] merlin (3 packages) (4.18-414)
71f7414 
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 voodoos mentioned this pull request Nov 26, 2024
Merged
Lucccyo pushed a commit to Lucccyo/merlin that referenced this pull request Dec 13, 2024
@voodoos @Lucccyo
Change entry for ocaml#1854
6de826a
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
Backport to 414
Status: 4.18-414
Backport to 501
Status: Ready for backporting
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@liam923 @voodoos