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

Show the pos (file info) column for GoDecls FZF view #3362

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jonaustin
Copy link

For some reason the file info column is not displayed in the FZF GoDecls view.

This PR simply adds that column to the view.

The lack of file info can be seen in the screenshot here: #3110

The difference is simply e.g.:

main                    func

versus:

main                    func    |main.go:7:1| 

@bhcleek
Copy link
Collaborator

bhcleek commented Feb 19, 2022

Thank you for contributing.

Why is this needed?

@jonaustin
Copy link
Author

Because it's useful to know which file a function is from, but maybe I'm missing something?

@bhcleek
Copy link
Collaborator

bhcleek commented Feb 20, 2022

I don't see why that would be useful given that directories contain only a single package and function names are within a package a unique. You can navigate to the function by selecting it and you can see the function signature in the preview window.

@jonaustin
Copy link
Author

jonaustin commented Feb 20, 2022

That's fair -- I guess I got confused because the ctrlp version of this does include the the file attributes:
image

Apparently also had it in my head for some reason that GoDecls acted recursively over the directory tree which I see now was incorrect.

Thanks for the quick feedback, feel free to close this.

@bhcleek
Copy link
Collaborator

bhcleek commented Feb 20, 2022

That's an interesting point; it may make sense for the ctrlp and fzf integrations to be more consistent in their presentation.

Honestly, I'd lean more towards removing the file info from the ctrlp integration, because :GoDecls only operates on the current file and :GoDeclsDir only operates on a single directory and the file info takes up valuable screen real estate.

I'll give it some thought, but I'm interested to hear your perspective.

@jonaustin
Copy link
Author

That makes a lot of sense; happy to refactor this PR to instead remove it from the ctrlp autoload.

@bravoecho
Copy link

bravoecho commented Sep 26, 2022

Just a quick comment in support of providing additional context to the symbol names.

I often have to give up :GoDeclsDir when there are different implementations of the same symbol, for example different types implementing the same interface cannot be told apart in the results, their methods are all, for example String() string, and I don't know which one to select.

Another common situation is when the same symbol is present both in the actual package (say, mypkg), and in the ..._test package (mypkg_test). The ..._test package coexists in the same directory as the real one, and symbols with the same name across the two cannot be identified in the GoDeclsDir results. This is often the case when using generated mocks.

For example:
godeclsdir

@bhcleek
Copy link
Collaborator

bhcleek commented Sep 26, 2022

@bravoecho in those cases, the preview window that allows me to distinguish between the types that are implementing the methods. Are you not seeing the preview window on the right hand side?

@bravoecho
Copy link

Thank you @bhcleek for your message 🙂 Good point! I actually disabled all previews to avoid information overload. You make me realise that perhaps this is because I'm using the feature for navigating code, rather than for searching it.

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