nvim-telescope / telescope.nvim

Find, Filter, Preview, Pick. All lua, all the time.
MIT License
15.43k stars 824 forks source link

Poor quality matches when searching using lsp_dynamic_workspace_symbols #2104

Open JoseConseco opened 2 years ago

JoseConseco commented 2 years ago

Description

For example seaching for: mute with lsp_dynamic_workspace_symbols I get this order of results: image Often more relevant matches are even lower. I though it shows current document symbols first, but changing active buffer did not change a thing.

Can someone confirm this? (lsp_document_symbols seem to work ok )

I use following mapping:

local showWorkspaceSymbols = function ()
    local opts= {
        symbols = {
            "class",
            "function",
            -- "method",
        }
    }
    if vim.bo.filetype == "vim" then
        opts.symbols = { "function" }
    end
    require('telescope.builtin').**lsp_dynamic_workspace_symbols**(opts)
end  

vim.keymap.set( "n", "<F3>", showWorkspaceSymbols, { noremap = true, silent = true, desc="Show Workspace Symbols" } )

Neovim version

NVIM v0.8.0-dev-709-ge0f32abb1
Build type: Release
LuaJIT 2.1.0-beta3
Compiled by root@bartosz-manjaro

Operating system and version

Manjaro Linux

Telescope version / branch / rev

Latest build using Packer

checkhealth telescope

telescope: require("telescope.health").check()
========================================================================
## Checking for required plugins
  - OK: plenary installed.
  - OK: nvim-treesitter installed.

## Checking external dependencies
  - OK: rg: found ripgrep 13.0.0
  - OK: fd: found fd 8.4.0

## ===== Installed extensions =====

## Telescope Extension: `aerial`
  - INFO: No healthcheck provided

## Telescope Extension: `file_browser`
  - INFO: No healthcheck provided

## Telescope Extension: `fzf`
  - INFO: No healthcheck provided

## Telescope Extension: `media_files`
  - INFO: No healthcheck provided

## Telescope Extension: `sessions_picker`
  - INFO: No healthcheck provided

## Telescope Extension: `smart_history`
  - INFO: No healthcheck provided

## Telescope Extension: `vim_bookmarks`
  - INFO: No healthcheck provided

Steps to reproduce

  1. Run mapping defined in first section
  2. See full matched words being lower, than semi matched words.

Expected behavior

Mull matched words should have higher score

Actual behavior

LQ matches are too high

Minimal config

vim.cmd [[set runtimepath=$VIMRUNTIME]]
vim.cmd [[set packpath=/tmp/nvim/site]]
local package_root = '/tmp/nvim/site/pack'
local install_path = package_root .. '/packer/start/packer.nvim'
local function load_plugins()
  require('packer').startup {
    {
      'wbthomason/packer.nvim',
      {
        'nvim-telescope/telescope.nvim',
        requires = {
          'nvim-lua/plenary.nvim',
          { 'nvim-telescope/telescope-fzf-native.nvim', run = 'make' },
        },
      },
      -- ADD PLUGINS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
    },
    config = {
      package_root = package_root,
      compile_path = install_path .. '/plugin/packer_compiled.lua',
      display = { non_interactive = true },
    },
  }
end
_G.load_config = function()
  require('telescope').setup()
  require('telescope').load_extension('fzf')
  -- ADD INIT.LUA SETTINGS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
end
if vim.fn.isdirectory(install_path) == 0 then
  print("Installing Telescope and dependencies.")
  vim.fn.system { 'git', 'clone', '--depth=1', 'https://github.com/wbthomason/packer.nvim', install_path }
end
load_plugins()
require('packer').sync()
vim.cmd [[autocmd User PackerComplete ++once echo "Ready!" | lua load_config()]]
sgarcialaguna commented 2 years ago

Since you showed this with Python, I noticed that it works poorly with Python symbols. JavaScript works as expected. So it's not an universal issue.

JoseConseco commented 2 years ago

so the quality of results depends on lsp? I though it is telescope that sorts the results by fuzzy match. For local symbols results seem sorted ok: image

stevanmilic commented 2 years ago

@JoseConseco I'm having the same problem with pyright LSP, I make use of the telescope action: require("telescope.actions").to_fuzzy_refine that uses the search of LSP and pushes it to the native fuzzy matching.

I have it mapped to CTRL+F:

local actions = require("telescope.actions")
telescope.setup({
       ...
        mappings = {
            i = {
                ...
                ["<c-f>"] = actions.to_fuzzy_refine,
            },
        },
    },
})
JoseConseco commented 2 years ago

Thx, I'm not sure why mappig wont work in my config. But I changed the default dynamic workspace symbols sorter from: sorter = sorters.highlighter_only(opts),

To: sorter = sorters.get_fzy_sorter(opts),

in _lsp.lua file -> in lsp.dynamic_workspace_symbols = function(opts) function . Now it works way better.

sgarcialaguna commented 2 years ago

@stevanmilic works for me, but it's slightly annoying. @JoseConseco Can you show in more detail where and how you set the custom sorter? I'm fairly new to neovim.

sgarcialaguna commented 2 years ago

Oh, you mean you edited the actual source of Telescope. Works as a bandaid, but surely that's not the intended way?

JoseConseco commented 2 years ago

Its not optimal, but I do not think there is way to override sorter that is used by workspace_symbols picker. (unless there is way?)

lundberg commented 2 years ago

I debugged the fzf native extension to see when it got used and by which picker, and for me it turned out that it was used by the lsp_workspace_symbols picker, but not the lsp_dynamic_workspace_symbols 🤔.

This telescope config worked for me to manually specify the sorter for the dynamic lsp picker, i.e. forcing usage of the fzf sorter, and now gives me better sorted results in the lsp_dynamic_workspace_symbols...

local telescope = require("telescope")
local fzf_opts = {
  fuzzy = true,                    -- false will only do exact matching
  override_generic_sorter = true,  -- override the generic sorter
  override_file_sorter = true,     -- override the file sorter
  case_mode = "smart_case",        -- or "ignore_case" or "respect_case"
                                   -- the default case_mode is "smart_case"
}

telescope.setup {
  -- ...

  pickers = {
    -- Manually set sorter, for some reason not picked up automatically
    lsp_dynamic_workspace_symbols = {
      sorter = telescope.extensions.fzf.native_fzf_sorter(fzf_opts)
    },
  },
  extensions = {
    fzf = fzf_opts
  },

  -- ...
}
telescope.load_extension('fzf')
JoseConseco commented 2 years ago

@lundberg your solution works. I did no know we could override sorters like this. Also using: sorter = sorters.get_generic_fuzzy_sorter() works too, but its slower compared to fzf extension.

lundberg commented 2 years ago

Also using: sorter = sorters.get_generic_fuzzy_sorter() works too, but its slower compared to fzf extension.

Yes, tried that one as well, and agree that it is slower, but most important fzf alsop gives me "better" sorted results.

lundberg commented 2 years ago

@JoseConseco, maybe we should leave this issue open, since it should not be needed to manually set the sorter IMO.

Anyone knows why the lsp_dynamic_workspace_symbols picker doesn't respect the fzf _override_genericsorter setting?

dixslyf commented 1 year ago

Anyone knows why the lsp_dynamic_workspace_symbols picker doesn't respect the fzf _override_genericsorter setting?

I took a peek at the code, and it seems that it's because lsp_dynamic_workspace_symbols doesn't use the generic sorter, but rather a sorter called sorters.highlighter_only(...). Relevant line is here. It did originally use the generic sorter as seen in the original PR (#705) for the finder, but it was changed at some point. Doing a git bisect shows that it was changed in #1115, though I have no idea if it was intentional. Interestingly, if you use <c-space> to refine the search, then it does use the generic sorter, which does get overridden by fzf native.

im-n1 commented 1 year ago

@lundberg your solution made lsp_dynamic_workspace_symbols finally usable. I don't get it why these values are not default.

j-krl commented 1 month ago

Just wanted to add here that if you want to solve this problem without installing an extension, I've gotten good performance using the native get_fzy_sorter like so:

pickers = {
    lsp_dynamic_workspace_symbols = {
        sorter = sorters.get_fzy_sorter(),
    }
}

So glad I found this thread!

im-n1 commented 1 month ago

@j-krl have you noticed any performance difference in comparison with sorter = telescope.extensions.fzf.native_fzf_sorter(fzf_opts)?

j-krl commented 1 month ago

@j-krl have you noticed any performance difference in comparison with sorter = telescope.extensions.fzf.native_fzf_sorter(fzf_opts)?

Honestly didn't even try the fzf extension to keep things minimal, but I haven't noticed any performance issues. I think the main pro @lundberg talked about is that fzf actually gives better matches. get_fzy_sorter() isn't perfect but I've found it pretty good.

jamestrew commented 1 month ago

@lundberg your solution made lsp_dynamic_workspace_symbols finally usable. I don't get it why these values are not default.

The reason is that telescope is dynamically querying the language server on every input and simply returning the results. This is similar to the live_grep picker. Telescope isn't doing any sorting for either. But of course this relies on the language server returning results that match the prompt well and in the order of match quality.

But it looks like that's not a behavior we can rely on. There are so many unexpected behaviors/issues with this picker that language server dependent, I'm kinda in favor of deprecating this.

You should use the non-dynamic version if you're having issues enough to change the sorter.

I'll continue to investigate and explore what to do about this picker in the meantime.

j-krl commented 1 month ago

But of course this relies on the language server returning results that match the prompt well and in the order of match quality.

Yeah I'm pretty sure pyright was just returning the results in ascending order based on file name, so changing the sorter was the best option.

You should use the non-dynamic version if you're having issues enough to change the sorter.

I would happily do this except that none of the LSPs I use will even open because of #964, which has been closed. I rely on this dynamic version pretty heavily so it would be a shame to see it deprecated and have no working alternative.

jamestrew commented 1 month ago

Right, I have a feeling #964 is also language server related. They're both literally making the same request except that the dynamic version is constantly updating the request with a query value. They both have the same response processing as well.

Maybe the solution to both issue is flip my suggestion around, deprecate the non-dynamic lsp_workspace_symbols (or alias it to the dynamic one) and as suggested, use a proper sorter for the dynamic version.

j-krl commented 1 month ago

Maybe the solution to both issue is flip my suggestion around, deprecate the non-dynamic lsp_workspace_symbols (or alias it to the dynamic one) and as suggested, use a proper sorter for the dynamic version.

As long as I'm following correctly I think this is a better solution. The constant updating in the dynamic version feels more in line with the other uses of Telescope like ripgrep and find_files. You're saying that the non-dynamic version doesn't update as you type but just makes a single query that you have to submit before the searching happens?

jamestrew commented 1 month ago

You're saying that the non-dynamic version doesn't update as you type but just makes a single query that you have to submit before the searching happens?

Correct. It makes 1 request (with the provided query option or just a blank query by default) and let's you search through the results with a modified conf.generic_sorter sorter. But I don't see any guarantees about what/how the language server returns.

For instance, after exploring more (I don't personally use either workspace symbol pickers generally) I just noticed that with basedpyright (and possibly regular pyright), I get no results when using a blank query. This effectively makes lsp_workspace_symbols completely useless in this case. lsp_dynamic_workspace_symbols does yield results once I start giving it a valid query/prompt.