Skip to content

Generic list_or_jump items filtering mechanism #3401

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

Open
TmLev opened this issue Jan 22, 2025 · 5 comments · May be fixed by #3402
Open

Generic list_or_jump items filtering mechanism #3401

TmLev opened this issue Jan 22, 2025 · 5 comments · May be fixed by #3402
Labels
enhancement Enhancement to performance, inner workings or existent features

Comments

@TmLev
Copy link

TmLev commented Jan 22, 2025

Is your feature request related to a problem? Please describe.

Sometimes I work on a React codebase, where I frequently use require('telescope.builtin').lsp_definitions to go to a component's definition. Currently, lsp_definitions opens a picker with two options:

  1. The component's definition
Image
  1. The component's type definition
Image

I rarely need the second option and would like to see it only if there are no other options available (e.g., "native" elements like div or form). So I need to filter out certain patterns only if more than one result is available.

Describe the solution you'd like

The lsp_definitions function calls list_or_jump inside:

lsp.definitions = function(opts)
local params = client_position_params(opts.winnr)
return list_or_jump("textDocument/definition", "LSP Definitions", "builtin.lsp_definitions", params, opts)
end

The list_or_jump function already has some form of filtering (ignoring files if they match user-given patterns):

items = filter_file_ignore_patters(items, opts)

local function filter_file_ignore_patters(items, opts)
local file_ignore_patterns = vim.F.if_nil(opts.file_ignore_patterns, conf.file_ignore_patterns)
file_ignore_patterns = file_ignore_patterns or {}
if vim.tbl_isempty(file_ignore_patterns) then
return items
end
return vim.tbl_filter(function(item)
for _, patt in ipairs(file_ignore_patterns) do
if string.match(item.filename, patt) then
return false
end
end
return true
end, items)
end

While it helps with static patterns (meaning patterns that don't depend on any other conditions), it doesn't allow for arbitrary conditions while filtering.

I suggest adding a callback called filter_items that enables users to filter the vim.quickfix.entry[] list (called items in the list_or_jump implementation) to make it flexible for users who want to control what is presented in the picker.

Judging by how many "public" functions use list_or_jump internally, adding filter_items would extend all these "public" functions in a helpful way without making a lot of changes.

Describe alternatives you've considered

Any other form of "hooking" into Telescope's internals that decide what the picker will show.

Additional context

Possible implementation:

  1. For each function in builtin/init.lua that uses list_or_jump internally, add a new field

    ---@field filter_items FilterItemsCallback: callback to filter out quickfix entries that should be ignored

    The type itself:

    ---@alias FilterItemsCallback fun(items: vim.quickfix.entry[]): vim.quickfix.entry[]
    
  2. Add a default implementation of filter_items in case a user didn't provide one:

    opts.filter_items = opts.filter_items or function(items)
        return items
    end
  3. Call opts.filter_items after other filtering:

    items = apply_action_handler(action, items, opts)
    items = filter_file_ignore_patters(items, opts)
    
    items = opts.filter_items(items) -- <- NEW

How to use:

map('gd', function()
  require('telescope.builtin').lsp_definitions {
    ignore_items = function(items)
      -- Only ignore React type definitions if there is more than one result.
      if #items > 1 then
        items = vim.tbl_filter(function(item)
          return not item.filename:find '@types/react/index.d.ts'
        end, items)
      end

      return items
    end,
  }
end, '[G]oto [D]efinition')

I can create a PR quickly (I already have a working version locally).

@TmLev TmLev added the enhancement Enhancement to performance, inner workings or existent features label Jan 22, 2025
@TmLev TmLev linked a pull request Jan 22, 2025 that will close this issue
4 tasks
@jamestrew
Copy link
Contributor

I'm leaning on completely gutting some/most of our lsp picker code and opting to leverage the on_list option of vim.lsp.ListOpts on many of the vim.lsp.buf.* functions.

We're needlessly maintaining a ton of duplicate code from neovim src, manually making server requests and such.

@TmLev
Copy link
Author

TmLev commented Feb 24, 2025

So how can I solve the issue without my suggested addition?

@jamestrew
Copy link
Contributor

Sorry, I didn't mean to close the issue.

You can have a working solution locally? Is that just a fork?

@jamestrew jamestrew reopened this Feb 24, 2025
@TmLev
Copy link
Author

TmLev commented Feb 24, 2025

I have a working solution in #3402

@jamestrew
Copy link
Contributor

Right right.
Let's put a pin on this for the time being.
Like a mentioned, I'd like to refactor the telescope lsp code significantly but I'd like to get other maintainers to give their inputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to performance, inner workings or existent features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants