Skip to content

Improve location of some custom operators #1917

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

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

Conversation

WardBrian
Copy link

This (maybe?) closes #1915.

It adds the same test as #1916, plus a:

  • Definite fix for :=
    • Just lex this as an operator.
  • Definite fix for reified custom indexing (e.g, something like ( .@(;..)<- ))
    • We lex this as DOTOP SEMI DOTDOT ... and detect it in the driver code
  • Heuristic fix for other custom indexing
    • When we see a DOTOP in the wild, we assume it is single-indexing without assignment and complete it accordingly. E.g., the code name.@( (* whatever here ... *) will get turned into .@ ( ) by reconstruct_id, even if we'd like to also detect things like ; or <- to disambiguate.
    • This is obviously wrong-in-general, but I think it's more useful than the current behavior, since users are encouraged to always define this variant if using the custom indexing, and they're often near each other. I also don't know if it can be done any better, since truly disambiguating requires I think arbitrary amounts of parsing?

The test shows both the cases that don't work before (see existing output in #1917) and the cases that "work" but produce incorrect locations due to the third bullet point above.

@WardBrian
Copy link
Author

Fuzzy CI diff looks all intentional to me

@voodoos
Copy link
Collaborator

voodoos commented Apr 2, 2025

Thanks for the patch @WardBrian. I will review it when I get some spare cycles (it might take some time..).

@voodoos voodoos self-assigned this Apr 2, 2025
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.

Go-to-definition does not work on custom operators
2 participants