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

Open references if definition result has the same range than clicked offset #531

Open
InSyncWithFoo opened this issue Sep 20, 2024 · 8 comments · May be fixed by #533
Open

Open references if definition result has the same range than clicked offset #531

InSyncWithFoo opened this issue Sep 20, 2024 · 8 comments · May be fixed by #533
Labels

Comments

@InSyncWithFoo
Copy link
Contributor

Currently Ctrl + Click always send textDocument/definition, which might not be ideal when the definition itself is clicked. LSP4IJ integrators should be able to configure that programmatically.

(Filed as requested at this issue.)

@CppCXY
Copy link
Contributor

CppCXY commented Sep 20, 2024

in VSCode, when you ctrl + click on a reference, it first sends a textDocument/definition request. If the returned range overlaps with the clicked position, it then sends a textDocument/references request.

@CppCXY
Copy link
Contributor

CppCXY commented Sep 20, 2024

test

@angelozerr
Copy link
Contributor

Thanks so much @CppCXY for your information.

I see now more the benefit to consume references.

@InSyncWithFoo is it the same usecase in the reported issue from your project?

@InSyncWithFoo
Copy link
Contributor Author

Yes, that's probably the best behaviour.

@angelozerr
Copy link
Contributor

angelozerr commented Sep 20, 2024

Yes, that's probably the best behaviour.

Indeed, but do you think it is the behavior that user would like to have?

@InSyncWithFoo
Copy link
Contributor Author

Of course. They want a panel; we should give them a panel.

@angelozerr angelozerr changed the title Allow configuring which request to send on Ctrl + Click Open references if definition result has the same range than clicked offset Sep 21, 2024
angelozerr added a commit to angelozerr/lsp4ij that referenced this issue Sep 21, 2024
@angelozerr
Copy link
Contributor

@InSyncWithFoo after debugging Java support in IJ, I undertand more how the go to declaration is working.

There are 2 means to manage go to declaration:

  • extends GotoDeclarationHandler like we did with LSPGotoDeclarationHandler. The advantage to use that is if there are some custom PsiFile (it is your case @InSyncWithFoo I think), the go to declaration will take the priority and if another plugin which manages go to declaration like PyCharm I think,the LSP go to definition will work. The inconevinant to use this GotoDeclarationHandler are:
  • using the PsiFile to collect declarations / references. In the LSP context, we have no custom PsiFile BUT it is possible to get declarations / referernces by implementing PsiSymbolDeclarationProvider / ImplicitReferenceProvider. But there are 2 problems:
    • ImplicitReferenceProvider#getImplicitReferences is called every time (when you do an hoverfor instance), so if we implement textDocument/definition in the getImplicitReferences, it will consume textDocument/definitionevery time. In our case we need to execute getImplicitReferences only when a GoToAction is executed. So we could implement an action listener before/after to enable/disable the call of getImplicitReferences
    • ImplicitReferenceProvider#getImplicitReferences is called only if PsiFile doesn't return references from teh given offset, it means in your case @InSyncWithFoo that you will loose teh LSP go to definition if the hyperlinked PsiElement is done.

What do you think about that @InSyncWithFoo : loosing go to definition (but I think PyCharm should work good) but benefit with Show references panel (and for other language the proper hyperlinked). For many language I think using ImplicitReferenceProvider#getImplicitReferences will be very nice,but before doing that I would liketo know your feeling. If you don't want to loose the LSP go to definition, perhaps we should provide an API to know if the language server must use GotoDeclarationHandler or ImplicitReferenceProvider.

@InSyncWithFoo
Copy link
Contributor Author

Either seems suboptimal. Hyperlinking is a small problem, so I slightly prefer the first in this "choose your poison" situation.

I'm not familiar with these extension points though. Take this comment only at face value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants