Description

This PR resolves a bug reported in #3118 in the list_or_jump function of the find_references feature for the default option "excluding the current position". Previously, the function incorrectly assumed that the locations and items tables were in the same order, which is not the case as vim.lsp.util.locations_to_items sorts items. This resulted in incorrect exclusions, affecting the functionality when handling multiple references (wrong exclusion) or a single reference (not jumping at all).

Changes Introduced

• Implemented a sorting mechanism for locations based on the order of items returned by vim.lsp.util.locations_to_items.
• The sorting is based on a stable identifier generated from both locations and items, ensuring that they match correctly by index.

Rationale

The chosen method leverages the output from vim.lsp.util.locations_to_items to ensure that locations are reordered to match the sorted items. This approach treats the Neovim utility as a black box and only assumes the stability of location and item type definitions, such as lsp.Location and lsp.LocationLink, so that it won't depend on the exact implementation of the utility.

The alternative approach would be replicating the sorting logics in vim.lsp.util.locations_to_items, but that would break the abstraction of neovim utility and increases risk of the implementation breaking due to internal changes in the vim.lsp.util.locations_to_items function.

This update ensures that the list_or_jump function behaves as expected, particularly in scenarios where the current location should be excluded from find_references results.

Testing

Use the same config in #3118.

Below is a minimal example that tests the functionality.

package main

import "fmt"

func main() {
	hello()

	a := 1
	fmt.Println(a)
	fmt.Println(a + 1)
}

func hello() {
	fmt.Println("Hello, World!")
}

It now correctly jumps to the reference in the above example for single reference hello. Before it does not work.

It also properly lists both references for a for the case of multiple references.

Checklist

• My code follows the style guidelines of this project (stylua)
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (lua annotations)

Just got started using Neovim last Friday. Still a newbie. Sorry if some part of the PR is not up to best practice. Feedback more than welcomed!

Thank you guys so much for making these killer plugins.

Wondering what else I need to do to get the PR merged

4 days open, give me at least time to look at it and try to understand the problem why it doesnt work. this code has been modified much more often than I would like and there is always the next PR. i actually need to think about this and do some testing. be patient, i will look at it. Thanks for understanding :)

4 days open, give me at least time to look at it and try to understand the problem why it doesnt work. this code has been modified much more often than I would like and there is always the next PR. i actually need to think about this and do some testing. be patient, i will look at it. Thanks for understanding :)

To give you some context, these might be useful:

• Initial "fix" that also contained the bug this PR aims to solve (thanks @odysseus0 for cleaning up my mess!) in Fix jump regression in LSP references action handler #3091
• subsequent refactor in refactor(lsp): simplify list_to_jump #3099
• which was later reverted in Revert "refactor(lsp): simplify list_to_jump" #3101 because it depended on nightly.

My take is that maybe #3099 maybe should be merged again, because just a few days after it was reverted 0.10 was released with the changes it depended on.

Again it will just take a couple more days to get this issue fixed. #3099 will not be merged again, we still have 0.9 support.

Don't mean to rush you at all! Just trying to figure out if there is something else I can or need to do on my end! Thank you so much for all your work.

Sorry @Conni2461 I can take this one considering I sort of started this mess.

I'm thinking I want to implement something closer to #3099 but since we don't have access to the user_data field to get Location|LocationLink info, we can just build this data ourselves at least as a carryover till we drop 0.9 support.

--- convert `item` type back to something we pass to `vim.lsp.util.jump_to_location`
---@param item vim.lsp.util.locations_to_items.ret
---@param offset_encoding string|nil utf-8|utf-16|utf-32
---@return lsp.Location # not exactly the same, only important parts for `jump_to_location`
local function item_to_location(item, offset_encoding)
  local location = {
    uri = vim.uri_from_fname(item.filename),
    range = {
      start = {
        line = item.lnum,
      },
    },
  }

  -- TODO: real implementation will need to use `item.line` and offset_encoding to
  -- get the real start.character value
  location.range.start.character = item.col
  return location
end

-- ...
-- ...
-- many lines later where to use `jump_to_location

      local location = item_to_location(item, offset_encoding)
      vim.lsp.util.jump_to_location(location, offset_encoding, opts.reuse_win)
    else
      pickers
        .new(opts, {

I think this could be simpler than sorting Location|LocationLink items to match the order of items. And it let's us generally simplify the action_handler code.

I can flesh out this idea myself a little bit later tonight.

I can flesh out this idea myself a little bit later tonight.

here it is: #3137

🤣 jesus I am being such an idiot. If I can generate the unique 1-1 mapping between them, that means I have enough information to just reconstruct the whole thing. Yes this approach makes more sense.

would close this in favor of #3137 its the simpler implementation.

still thanks for looking into the issue :)