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

POC for fzf wrap for GoImplements #3320

Closed
wants to merge 2 commits into from
Closed

Conversation

kerma
Copy link

@kerma kerma commented Nov 5, 2021

This is a draft PR for #3319

Code is largely based on GoDecls implementation.

It's not fully tested, needs some code cleanup and has a problem where fzf popup window is out of focus after GoImplements is triggered. Something to do with go#lsp#Implements async behavior I'm guessing.

EDIT: I haven't yet figured out the focus issue, but the insert mode is related to this. fzf calls startinsert, but due to wrong focus this affects the main window.

vim-go-implements-fzf.mov

@bhcleek
Copy link
Collaborator

bhcleek commented Nov 6, 2021

Thank you for the contribution.

I'd like to find a more general solution and tie this to the g:go_list_type and g:go_list_type_commands instead of to the mode; the mode is for what's used to get the information, while g:go_list_type and g:go_list_type_commands are for how the results are displayed.

@kerma
Copy link
Author

kerma commented Nov 7, 2021

I'm not sure whether my initial approach could work at all. Calling fzf#run() via jobstart creates the focus issue and I couldn't find an easy maintainable solution to get the focus back. It would probably need changes in fzf#run() to add that support.

Due to that general solution via g:go_list_type is probably also quite big change as I don't think you can add the fzf wrap for go#lsp methods at all.

I rewrote the code to use gopls via command line instead. It's now very similar to GoDecls with motion.

EDIT: I've also changed the config from mode to listtype. But due to this being gopls via Exec() now it's more a mode than listtype?

@bhcleek
Copy link
Collaborator

bhcleek commented Nov 7, 2021

You're right; your initial approach won't work with the asynchronous nature it had.

Changing :GoImplements to be a synchronous call is going to have performance problems in large repositories; if we're going to make this work, then the call needs to remain asynchronous.

I believe it is possible to change this to work more generally. Thank you for your contribution. I'm not interested in doing a one-off for :GoImplements to support fzf, though.

@kerma kerma closed this Feb 14, 2023
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