vim-go icon indicating copy to clipboard operation
vim-go copied to clipboard

POC for fzf wrap for GoImplements

Open kerma opened this issue 3 years ago • 3 comments

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.

https://user-images.githubusercontent.com/735108/140577115-26448ac3-d5fd-47a7-9a0a-24eb0373b144.mov

kerma avatar Nov 05 '21 21:11 kerma

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.

bhcleek avatar Nov 06 '21 23:11 bhcleek

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?

kerma avatar Nov 07 '21 12:11 kerma

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.

bhcleek avatar Nov 07 '21 17:11 bhcleek