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

Picker:delete_selection doesn't work if entry_maker returns nil #3265

Closed
cameronr opened this issue Aug 22, 2024 · 0 comments · Fixed by #3266
Closed

Picker:delete_selection doesn't work if entry_maker returns nil #3265

cameronr opened this issue Aug 22, 2024 · 0 comments · Fixed by #3266
Labels
bug Something isn't working

Comments

@cameronr
Copy link
Contributor

cameronr commented Aug 22, 2024

Description

Picker:delete_selection uses ipairs to iterate over the finder.results:

for result_index, result_entry in ipairs(self.finder.results) do
if vim.tbl_contains(delete_selections, result_entry) then
table.insert(selection_index, result_index)
end
end

If the entry_maker for a one shot job returns nil for one of the lines, it makes the indices discontinuous:

for line in stdout:iter(false) do
num_results = num_results + 1
if num_results % await_count then
async.util.scheduler()
end
local entry = entry_maker(line)
if entry then
entry.index = num_results
end
results[num_results] = entry
process_result(entry)
end

Which means the ipairs iteration won't see any of the results after the nil.

Having the entry_maker return {} instead isn't ideal because then the results count is off (i.e. the empty rows aren't displayed but they still show up in the count of results).

I think the correct behavior is for Picker:delete_selection to use pairs instead.

Neovim version

NVIM v0.10.1
Build type: Release
LuaJIT 2.1.1720049189

Operating system and version

macos 14.6

Telescope version / branch / rev

telescope 0.1.8

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 14.1.0
- OK fd: found fd 10.1.0

===== Installed extensions ===== ~

Telescope Extension: `advanced_git_search` ~
- No healthcheck provided

Telescope Extension: `fzf` ~
- OK lib working as expected
- OK file_sorter correctly configured
- OK generic_sorter correctly configured

Telescope Extension: `neoclip` ~
- No healthcheck provided

Telescope Extension: `session-lens` ~
- No healthcheck provided

Telescope Extension: `ui-select` ~
- No healthcheck provided

Steps to reproduce

  1. Use minimal config
  2. Run touch .repro/data/nvim/sessions/ax.vim to create an empty session file (do this in the directory used for the repro). This will cause the entry_maker to return nil for this file
  3. run nvim -u repro.lua
  4. Save a session via :SessionSave
  5. Open session picker via :SessionSearch
  6. Try to delete the session you just created (the only one in the picker) with

Expected behavior

The session is deleted and the picker is updated

Actual behavior

Nothing happens because Picker:delete_selection doesn't find the result so it doesn't call the callback to perform the delete

Minimal config

local root = vim.fn.fnamemodify("./.repro", ":p")

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
  vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
local lazypath = root .. "/plugins/lazy.nvim"
if not vim.uv.fs_stat(lazypath) then
  vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "https://github.com/folke/lazy.nvim.git",
    lazypath,
  })
end
vim.opt.runtimepath:prepend(lazypath)

-- install plugins
local plugins = {
  {
    "nvim-telescope/telescope.nvim",
    dependencies = {
      "nvim-lua/plenary.nvim",
    },
    config = function()
      -- ADD INIT.LUA SETTINGS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
      require("telescope").setup({})
    end,
  },
  {
    "rmagatti/auto-session",
    lazy = false,
    opts = {},
  },
}

require("lazy").setup(plugins, {
  root = root .. "/plugins",
})
@cameronr cameronr added the bug Something isn't working label Aug 22, 2024
cameronr added a commit to cameronr/telescope.nvim that referenced this issue Aug 22, 2024
ipairs won't find find all results if entry_maker happens to return
nil for a line.
cameronr added a commit to cameronr/auto-session that referenced this issue Aug 22, 2024
Sometimes deleting a session from the the Telescope picker wasn't
working. It happens when we have extra command files that are filtered
out from the list. Those files break some code in Telescope that finds
the item we want to delete in the picker results. More detail is here:

nvim-telescope/telescope.nvim#3265

We work around the issue by having the Telescope entry maker returning
{} instead of nil. That's not perfect but better than not being able to
delete/
cameronr added a commit to cameronr/auto-session that referenced this issue Sep 12, 2024
Telescope merged my fix for:
nvim-telescope/telescope.nvim#3265

So now we can return nil to have a correct count and still have delete
sessions from the Telescope picker work
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.

1 participant