nvim-telescope / telescope.nvim

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

Wrong cursor position after actions.close #2849

Open Ajnasz opened 10 months ago

Ajnasz commented 10 months ago

Description

The cursor position is off by 1 character after calling actions.close() https://github.com/nvim-telescope/telescope.nvim/blob/87e92ea31b2b61d45ad044cf7b2d9b66dad2a618/lua/telescope/actions/init.lua#L389

It causes issue when we try to change the text at the cursor after an entry has been selected in treesitter.

My suggested solution is to remove the + 1 from the line above.

Neovim version

NVIM v0.10.0-dev-2037+ge09adfdcf
Build type: RelWithDebInfo
LuaJIT 2.1.1703358377

Operating system and version

Debian Bookworm

Telescope version / branch / rev

master

checkhealth telescope

telescope: health#telescope#check

Checking for required plugins ~
- OK plenary installed.
- WARNING nvim-treesitter not found. (Required for `:Telescope treesitter`.)

Checking external dependencies ~
- OK rg: found ripgrep 13.0.0
- WARNING fd: not found. Install [sharkdp/fd](https://github.com/sharkdp/fd) for extended capabilities

Steps to reproduce

Open nvim with the provided minimal config

Add some text to the buffer, eg: ##

Place the cursor between the two characters (#|#), then type the :OpenPicker, then select the "Insert text" entry by pressing enter.

Expected behavior

I expected to add the inserted text between the two characters, so to have #inserted text# text in the buffer

Actual behavior

The text added after the cursor, ##inserted text

If you use the user command :InserthTheText, it will add the text to the correct place.

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

    local function execute_commands(prompt_bufnr)
        local action_state = require("telescope.actions.state")
        local selection = action_state.get_selected_entry(prompt_bufnr)
        local cmd = selection.value
        local actions = require("telescope.actions")

        actions.close(prompt_bufnr)

        cmd()

        return true
    end

    local function insert_text()
        vim.api.nvim_put({"inserted text"}, "", false, true)
    end

    local function entry_maker(entry)
        return entry
    end

    local function new_finder(opts)
        local finders = require("telescope.finders")
        return finders.new_table({results = opts.results, entry_maker = entry_maker})
    end

    local function mappings(prompt_bufnr, _)
        local function exec()
            return execute_commands(prompt_bufnr)
        end
        local actions = require("telescope.actions")
        actions.select_default:replace(exec)
        return true
    end

    local function custom_command_picker()
        opts = {
            results = {
                {
                display = "Insert text",
                value = insert_text,
                ordinal = "Insert text",
                }
            }
        }

        local pickers = require("telescope.pickers")
        local conf = (require("telescope.config")).values
        local picker = pickers.new(opts, {
            finder = new_finder(opts),
            attach_mappings = mappings,
        })
        return picker:find()
    end

    vim.api.nvim_create_user_command("OpenPicker", custom_command_picker, {})
    vim.api.nvim_create_user_command("InsertTheText", insert_text, {})
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()]]
Ajnasz commented 10 months ago

I found, that the builtin commands would work incorrectly if I remove the + 1, but I don't understand why. It uses nvim_feedkeys to execute a command, before the feedkeys call the cursor column is + 1. But if I type the keys myself :echo getpos('.') | InsertTheText, the getpos call returns the actual cursor position and the text is still inserted to the right place.

jamestrew commented 10 months ago

Thanks for reporting. This is something we've been tracking and struggling to get a clean fix. I opened a PR with a potential fix for it but there's a ton of edge cases and previous regressions I want to avoid so I'm going to take some time with.

jamestrew commented 10 months ago

If you can give the PR a try, that'll be helpful.

Ajnasz commented 10 months ago

If you can give the PR a try, that'll be helpful.

Sure, it's #2850, right?

My code works correctly with it, but executing a command with Telescope commands will be buggy. The strange thing is that the position is wrong if we execute a user command with vim.api.nvim_feedkeys, but it's good if we use vim.cmd.

I'm using my runcmd plugin for testing. (Simplified version of it attached to the issue in the Minimal config section)

These are the lua functions I use:

local function pos()
  local curpos = vim.fn.getpos(".")
  return vim.api.nvim_put({vim.inspect(curpos)}, "", false, true)
end
vim.api.nvim_create_user_command("Pos", pos, {})

local function feed_pos()
  local cr = vim.api.nvim_replace_termcodes("<cr>", true, false, true)
  return vim.api.nvim_feedkeys((":Pos" .. cr), "t", false)
end
local function exec_pos()
  return vim.cmd("Pos")
end

vim.api.nvim_create_user_command("FeedPos", feed_pos, {})
vim.api.nvim_create_user_command("ExecPos", exec_pos, {})

vim.g.runcmd_commands = {
    runcmd.new_command("FeedPos", feed_pos),
    runcmd.new_command("ExecPos", exec_pos),
}

Calling the ExecPos from telescope inserts the text to the correct place Calling the FeedPos from telescope inserts the text to a wrong position, shifted to the left by 1 character, and also the inserted text contains a wrong column.

If I print the current position before and after calling the FeedPos (my original fennel code or you can check the generated lua code) I get the right position. So the position is wrong only inside the call of nvim_feedkeys.

jamestrew commented 10 months ago

Pushed another commit. Should fix this as well. Just throwing vim.schedule around everywhere :laughing:

Ajnasz commented 10 months ago

Pushed another commit. Should fix this as well. Just throwing vim.schedule around everywhere 😆

That works, thanks.

I'm still worried a bit about regressions. Do you have an idea what is happening here, where is the wrong position coming from?

jamestrew commented 10 months ago

My theory is that we were setting cursor positions or allowing subsequent commands that set cursor positions too early in the vim event loop before it was "safe" to do so.

Conni (another maintainer) and I will continue to use the PR branch for a few days to get some level of confidence then merge it into master and allow others riding the master branch to catch anything we might've missed.

seflue commented 5 months ago

@jamestrew I stumbled upon this issue, because I also observe this wrong behavior while playing through the Telescope tutorial. Is there any chance, that #2850 will get merged soon?

jamestrew commented 5 months ago

Original attempt at fixing this had some issues. I might have some new ideas for this though. I'll try again.