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

Poor quality matches when searching using lsp_dynamic_workspace_symbols #2104

Open
JoseConseco opened this issue Jul 28, 2022 · 21 comments · May be fixed by #3250
Open

Poor quality matches when searching using lsp_dynamic_workspace_symbols #2104

JoseConseco opened this issue Jul 28, 2022 · 21 comments · May be fixed by #3250
Labels
bug Something isn't working

Comments

@JoseConseco
Copy link

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()]]
@JoseConseco JoseConseco added the bug Something isn't working label Jul 28, 2022
@sgarcialaguna
Copy link

sgarcialaguna commented Aug 17, 2022

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
Copy link
Author

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
Copy link

stevanmilic commented Aug 17, 2022

@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
Copy link
Author

JoseConseco commented Aug 17, 2022

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
Copy link

@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
Copy link

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

@JoseConseco
Copy link
Author

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
Copy link

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
Copy link
Author

@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
Copy link

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
Copy link

lundberg commented Aug 23, 2022

@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_generic_sorter setting?

@JoseConseco JoseConseco reopened this Aug 23, 2022
@dixslyf
Copy link

dixslyf commented Oct 25, 2022

Anyone knows why the lsp_dynamic_workspace_symbols picker doesn't respect the fzf override_generic_sorter 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
Copy link

im-n1 commented Jan 16, 2023

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

ditsuke added a commit to ditsuke/dotfiles that referenced this issue Mar 12, 2023
Hooks up fzf to the picker, which for some reason does not happen by default.

Reference: nvim-telescope/telescope.nvim#2104 (comment)
@j-krl
Copy link

j-krl commented Aug 8, 2024

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
Copy link

im-n1 commented Aug 8, 2024

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

@j-krl
Copy link

j-krl commented Aug 8, 2024

@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
Copy link
Contributor

@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
Copy link

j-krl commented Aug 11, 2024

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
Copy link
Contributor

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
Copy link

j-krl commented Aug 12, 2024

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
Copy link
Contributor

jamestrew commented Aug 12, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants