The default maximum result display count seems to have been mistakenly set to `250`

[leonasdev]

Description

In: https://github.com/nvim-telescope/telescope.nvim/blob/20bf20500c95208c3ac0ef07245065bf94dcab15/lua/telescope/pickers.lua#L561C1-L565C44 , the comment seems to indicate that the default value for max_result should be 10000.

But in:

telescope.nvim/lua/telescope/pickers.lua

Line 319 in 20bf205

__scrolling_limit = tonumber(vim.F.if_nil(opts.temp__scrolling_limit, 250)),

, __scrolling_limit is set to 250 when opt.temp__scrolling_limit is not present which seems wrong.

When users search for keywords, they may scroll beyond 250 entries to find a specific item. If the actual number of entries exceeds 250, users might mistakenly believe that they have thoroughly searched for the keywords.

And traversing 250 entries is more likely compared to navigating through 10000 entries.

So, I believe setting max_result to 10000 is more reasonable, or alternatively, allowing users to configure this value based on their performance preferences would be a better option.

Neovim version

NVIM v0.9.2
Build type: Release
LuaJIT 2.1.1692716794

Operating system and version

Ubuntu 22.04

Telescope version / branch / rev

master

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.3.1

Steps to reproduce

1. nvim
2. :Telescope highlights
3. The prompt shows the total of result may beyond 250 (like 500 or something)
4. When you scolling results list, you'll find that not all results is on the Results list.

Expected behavior

The max_result value should set to something like 10000 not 250, or alternatively, allowing users to configure this value based on their performance preferences would be a better option.

Actual behavior

The max_result seems accidentally set to 250, whcih users might mistakenly believe that they have thoroughly searched for the keywords.

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()]]

[staticssleever668]

I've tried the option for a few days and noticed an issue - if you call the 'restore' picker with a different temp__scrolling_limit value than the previous picker, instead of the previously selected line the very last one is always selected.

[Conni2461]

We actually changed it from 10000 down to 250 because of performance issues. Having a scrolling limit that high means that we actually need to sort 10k items correctly on every input change (or at least we start the sorting). if its just 250 we can discard correctly sorting everything after 250 we only need to figure out whats inside that window.

Maybe 1000 is more reasonable.

For resume we could just load zhe same scroll limit

[mister-choo]

We actually changed it from 10000 down to 250 because of performance issues. Having a scrolling limit that high means that we actually need to sort 10k items correctly on every input change (or at least we start the sorting). if its just 250 we can discard correctly sorting everything after 250 we only need to figure out whats inside that window.

Maybe 1000 is more reasonable.

For resume we could just load zhe same scroll limit

Again, an option would be nice

[gzagatti]

I faced a similar issue when going through a list of diagnostics. The list was well beyond 250 items because the diagnostic was overly sensitive. Telescope let me scroll until item 250, but it wouldn't go past this number of items even though it displayed additional results at the bottom of the list. I know I could have filtered the results, but for this particular task I couldn't write a filter that would select only the results I want with confidence.

A suggestion would be to add a shortcut that the user can press to add all the results to the current list. This would be similar to a "load more..." button in some websites. That way, all the results would have a hard limit of 250 by default, but could be increase on a case-by-case by the user.

[develoopeer]

Any change of it?

I encountered a similar issue when tried to create a picker with a commit list containing more than 250 items and got an unresponsive item counter in the picker prompt. Is it possible to manually set max_result value for specific picker? If not will this feature be implemented?

[anthony-crystalpeak]

Just chiming in that I also run into this when using grep picker to navigate code bases, perhaps the scroll limit could be picker-specific ?

[davvid]

There's a workaround hack I've been using in telescope-git-grep.nvim to lift the limit.

https://github.com/davvid/telescope-git-grep.nvim/blob/d2e1322934e334ee9eb8e838c30f3afb3e7a730e/lua/git_grep/init.lua#L68

The opts table in that code block eventually gets passed to a picker. This is not a long-term solution, but it'll work "for now" if you just want to get things working.

While I'm happy that this workaround has been working for the past few months, I understand that it may stop working at any time. I'd very much appreciate if the temp__scrolling_limit field were promoted to be an officially-supported scrolling_limit option.

[davvid]

#3471 exposes a scrolling_limit option in case y'all are receptive to supporting it.

[develoopeer]

Hello @davvid.

I tried your solution and it works for me. I just wonder why have you increased default scrolling_limit to 500. Will it negatively affect performance? Maybe to stay more consistent scrolling_limit should should not be changed any further.

And the question for maintainers, Will this PR ever be merged, i think custom scrolling_limit for specific picker is a good way to go and should be available by default.

[davvid]

Thanks @develoopeer -- there wasn't a strong reason why I upped the limit. It was primarily because I figured that 250 was simply a conservatively low number and that 500 would probably be just fine as a default.

LIkewise, I even considered bumping it up to 1000 since I figured that it would eliminate another swath of scenarios where people feel that they need to configure things.

That said, I think it'd be better for me to undo that change and keep it at 250 so that the code is closer to its original intent. If someone later thinks that the defaults can be changed then that could be discussed separately from the topic of making scrolling_limit an officially supported option.

I just force-pushed over the PR branch to drop the commit where the limit was changed. Thanks for the sug.