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

Use telescope for goto implementation #404

Merged
merged 2 commits into from
Sep 7, 2023

Conversation

e-aakash
Copy link
Contributor

Current key binding for goto implementation uses the default lsp functionality, which opens the list of implementations in new buffer.

Since the number of implementations is usually more than 1, using telescope.builtin.lsp_implementation is better since it shows the result in a dialog box and also shows preview.

@feoh
Copy link
Collaborator

feoh commented Aug 20, 2023

Hi!

I really like this idea! Could you please add detailed instructions for testing this to the PR?

Like, if I pull this change into my fork and run Neovim, what do I need to do to test your change? What configuration assumptions does the change make that might not be suared by all users?

Thanks for the contribution. Looking forward to working with you to get this merged.

init.lua Outdated Show resolved Hide resolved
Fix typo in original.

Co-authored-by: Luis G Estrades <luisgarciaestrades@gmail.com>
@feoh
Copy link
Collaborator

feoh commented Aug 21, 2023

Gotta say, I know this was a typo, we all make them, and I don't want to beat up on anyone for making a contribution, but - how did you test this? I copied your change locally and was wondering why nothing was happening :)

@e-aakash
Copy link
Contributor Author

e-aakash commented Aug 22, 2023

Hi, sorry for delay in reply.

Like, if I pull this change into my fork and run Neovim, what do I need to do to test your change? What configuration assumptions does the change make that might not be suared by all users?

The assumption is user is working on language which is multiple implementation for a thing (like java interfaces) and has the corresponding lsp installed.

Previous behaviour: on pressing gI a buffer is opened listing implementaiotns
New behaviour: on pressing gI, telescope popup/floating window opens with implementations, along with preview

how did you test this

I was working two different machines, and made this pull request by manually typing the change, thinking it was small. I should have tested it again in the primary machine (forgot to test in the excitement of raising PR). Thanks for the comment and quick fix. I will keep in mind to test before raising pull request.

@feoh
Copy link
Collaborator

feoh commented Aug 29, 2023

Yes please. Let me know when this is thoroughly tested and ready for inclusion.

@feoh
Copy link
Collaborator

feoh commented Sep 5, 2023

@e-aakash Are you still interested in getting this PR merged or should I close it?

If I don't hear from you in a week or two I'll do that. I know what it's like to get busy :)

@feoh
Copy link
Collaborator

feoh commented Sep 7, 2023

OK I tested this a bunch and read the documentation for telescope.builtin.lsp_implementation.

This makes sense to me and it seems to work so I'm going to merge it.

If anyone has feelings about this, we can always revert.

@feoh feoh merged commit 6453352 into nvim-lua:master Sep 7, 2023
qiuye2015 pushed a commit to qiuye2015/fjp.nvim that referenced this pull request Dec 10, 2023
s-frick pushed a commit to s-frick/kickstart.nvim that referenced this pull request Jul 29, 2024
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.

3 participants