feat: goto definition

[juliamertz]

Hey, I've been working on an adapter for the Nix language, but since much of it is undocumented I often find myself looking up the source code. Having a keymap to jump to it's definition would make this plugin perfect for me.

I've added the possibility for adapters to provide a path and the position in the file to jump to, though there are still some caveats:

• fzf-lua doesn't accept regular keymaps so I set it to <C-s> instead of gd
• I wasn't able to get it working for the native picker or mini.pick
• This doesn't make sense for all languages; for example, the implementation for the go adapter goes to the package directory instead of the actual implementation (omitting get_definition is also an option in this case)

This might be out of scope for this plugin, so just let me know if you'd be interested in including this, thanks :)

[fredrikaverpil]

Hey @juliamertz very interesting idea, nice! 😄 I've been thinking along the same lines as you but haven't had time to explore this yet.

I'm absolutely open to adding something like this. Looks great so far!

A few things:

• You need to also add the respective get_definition entries in the main init.lua:
  • M.validate_adapter: { name = "get_definition", type = "function" },
  • M.override_adapter: get_definition = user_opts.get_definition or default_adapter.get_definition,
• Let's definitively make adapter.get_definition optional, yes. 👍
• Don't worry if a picker cannot support gd. We can add instructions on this in the README or similar.
• I don't think the Go adapter implementation is useful/working right now? I'm not sure how the Go adapter would use this... 🤔 Maybe adding a buffer (of filetype go), write the package into the buffer like package <name-of-package>, put the cursor on it and actually run the equivalent gotodef command using the picker of choice, like Snacks.picker.lsp_definitions() (vim.lsp.buf.definition() for native).

It would be great if you were able to add an adapter (or fix the Go adapter) which would take advantage of this properly (so we know the implementation actually works as intended).

[juliamertz]

Using goto definition on a package import opens a list of all files in that package (at least with gopls). This behavior is similar to running :e /my/directory, which is what the go adapter currently does.

Screen.Recording.2025-02-28.at.13.07.40.mov

I could add the adapter I made for Nix, but it depends on an external cli tool I wrote, which is still somewhat cumbersome to build, so I didn't feel it was ready to include yet.

[fredrikaverpil]

@juliamertz ahh, I see now.

On my end, if neo-tree is loaded, I get it to show the directory 👍

However, hmm, I wonder. Maybe it would be more useful (or in addition to this) to be able to do a "go to ref" instead, which would take you to the picker. Something like this (the database/sql package):

I could add the adapter I made for Nix, but it depends on an external cli tool I wrote, which is still somewhat cumbersome to build, so I didn't feel it was ready to include yet.

No worries. You can keep it for yourself for as long as you like. 😄 I think, however, I want the Go adapter to use your added functionality but as ergonomically as possible (meaning maybe "goto refs" is more usable). But I don't have the bandwidth to look into this until later next week or so.

[juliamertz]

Ah right I think I misunderstood, that seems a lot better!
I'll try to get it implemented sometime soon :)

[juliamertz]

I managed to cobble together something that kind of works, but it's all a bit hacky and it only works on a good day. I can push my changes if you want to take a shot at it when you have the time 😄

[fredrikaverpil]

@juliamertz sure, push and I can have a look 😄

[fredrikaverpil]

@juliamertz hey, this looks super nice, but I'd like to take a stab at making this a bit more robust if it's ok with you?

[fredrikaverpil]

Please have a look at what I did in my latest commit.

It's not perfect, but (on my machine) the picker is more reliably showing for the goto_definition go package action. It needs some polishing still, so to avoid having the temporary buffer lingering around etc. And not great to use vim.wait... but I think I can whip something up later on that.

But maybe more importantly for you (for your adapter):

• Modified the implementation and updated interfaces so to delegate all "gotodef logic" to the adapter. This means no filepath or row/col is returned back to godoc. Instead you (as adapter author) has full control over what you want to do on gd.
• Renamed the functions from get_definition to goto_definition as it seemed more appropriate 🤔 (again, ties into not returning anything back to godoc).
• Renamed the M.lsp_definitions to M.goto_definition as it seemed more fitting here too 🤔 ... and I made these into functions so to avoid errors if you don't have a picker dependency installed.

Can you still do what you want but with this interface?
If not, let me know what you are missing or what doesn't work well!

[fredrikaverpil]

Let me know if there's anything from the Go adapter you would like to reuse. I could expose it from some sort of utils module, or similar.

demo.mov

[fredrikaverpil]

Hm, I just noticed that when using fzf-lua, the require("fzf-lua").lsp_definitions() call doesn't seem to do anything for some reason. I can see it invokes the adapter's goto_definition function call and the LSP is attached and everything 🤔

If switching to the snacks or telescope picker, it just works... hm, weird.

[juliamertz]

Nice! these changes have made it way more reliable for me too, with some minor changes my adapter works again.

Only thing i can think of that might be useful to expose is something like godoc.config.open_window(), right now the go adapter replaces your current buffer on goto_definition while show_documentation opens a new split. For my nix adapter i just use :vsplit at the moment, but i think using the configured split type would be preferable.

I tested it with fzf-lua btw and it works just fine for me 🤔

[fredrikaverpil]

right now the go adapter replaces your current buffer on goto_definition while show_documentation opens a new split

Good catch, yes, this should be configurable similar to how you can choose between split or vsplit, I guess. I'll have a look and see where/how it makes the most sense to get that configured.

I tested it with fzf-lua btw and it works just fine for me 🤔

Is it possible that you have any fzf-lua opts registered elsewhere, or in your own adapter?
I just now noticed that the fzf-lua picker is always showing reliably on ctrl-s when there is more than one definition to go to. But then the cursor focus is not on the picker. When there is only one defintion to go to, the action executes inside the hidden window without focussing it (so it appears as if nothing happens). It might be that fzf-lua does not automatically focus the window in which it is running lsp_definitions(). 🤔
But it's somewhat peculiar we are seeing different behaviors?
I'm just using default dependencies = { "ibhagwan/fzf-lua", opts = {} } myself.

[fredrikaverpil]

@juliamertz I changed things around a bit and now the window.type option is respected for gotodef as well.

I also simplified the code and I'm no longer using a hidden buffer. The drawback is you see the momentary "dummy buffer" before the LSP has attached. On my end the fzf-lua picker now works as expected, and like the other pickers.

I was thinking about fzf-lua's ctrl-s. Do you think users would expect ctrl-v to also work, without having to set the window.type option?
And would users actually expect ctrl-s to open the regular docs in a split rather than get this new gotodef behavior? 🤔

[juliamertz]

Creating the window before the callback works well for me, this should help keep it a bit more uniform across adapters.

At the time I just chose an arbitrary keybind, since ctrl-d for definition is already used. I felt ctrl-s was the best alternative, but didn't really put much more thought into it. I don't think anyone would mind having extra binds to open different split types, but I'm using telescope so i don't really mind either way.

Weirdly with the latest commit, fzf-lua isn't working as expected for me anymore, and I am now seeing the exact behavior you described. I don't have any special options set i just call fzf_lua.setup({}). this could be a difference in version though as i use nix to manage my plugins, so they might be a bit older if you're on the latest commit.

[fredrikaverpil]

I felt ctrl-s was the best alternative

I agree, it's fine. Let's start out like this. I don't use fzf-lua either and let's have someone else pick up this ball 😄

Weirdly with the latest commit, fzf-lua isn't working as expected for me anymore, and I am now seeing the exact behavior you described.

Strange, oh well, it works here on my end. I'll just merge this in now, and we can always iterate.

Many thanks for your contribution on this!

[juliamertz]

No problem, thank you for your time and help! :)