-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Support fzf on GoDecls[Dir] #1437
Conversation
I just added some documentation, please also review that :D |
Thanks 👍 Seems to work well in my testing. Using
This message is somewhat confusing, since it should be more like "please install ctrlp.vim, fzf, or unite.vim" (unite.vim support was added in #1391, would be nice if Actually, the best would be to autodetect one of those plugins and just use it, and have |
@Carpetsmoker Unite is supported through a different command, I'll update the PR so it detects the tool and use it, preferring ctrlp.vim so we can keep the current behavior. Thanks for the feedback! |
Wouldn't it be possible to have I have no idea if this makes sense; I don't use any of these plugins myself :-) But it seems to me that this would make this command easier to use for people who have unite.vim installed? |
autoload/fzf/decls.vim
Outdated
let cmd = get({'ctrl-x': 'split', | ||
\ 'ctrl-v': 'vertical split', | ||
\ 'ctrl-t': 'tabe'}, a:str[0], 'e') | ||
execute cmd filepath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you want to escape this with fnameescape(filepath)
, otherwise it will break if the path contains characters (see :help fnameescape()
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh, nice to know, thank you!
I've updated the code.
@Carpetsmoker I think the integration with Unite exists to support people that use I just finished the logic for automatically picking between fzf and ctrlp. Please let me know what you think. Thanks for the feedback! |
Otherwise a path such as `~/go/src/a/space path/a.go` will error out. Also: - add `abort` to all functions so they will stop when an error occurs; - tweak docs a bit; - while I'm here update the ChangeLog.
Okay, did some more testing and works well; I did still encounter some problems when using paths with spaces in it. I fixed that in the above commit. I also tweaked the docs a wee bit in that.
If that's the standard way then it's fine 👍 |
@Carpetsmoker awesome, thank you very much! |
Revisiting #976.
This introduces a new configuration option (
g:go_decls_mode
). It's still missing documentation, but I wanted to open the PR anyway so I can get feedback on the implementation before moving forward with the documentation.