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

Wrong cursor position after actions.close #2849

Open
Ajnasz opened this issue Jan 5, 2024 · 9 comments · May be fixed by #2850
Open

Wrong cursor position after actions.close #2849

Ajnasz opened this issue Jan 5, 2024 · 9 comments · May be fixed by #2850
Labels
bug Something isn't working

Comments

@Ajnasz
Copy link

Ajnasz commented Jan 5, 2024

Description

The cursor position is off by 1 character after calling actions.close()

pcall(a.nvim_win_set_cursor, original_win_id, { original_cursor[1], original_cursor[2] + 1 })

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 Ajnasz added the bug Something isn't working label Jan 5, 2024
@Ajnasz
Copy link
Author

Ajnasz commented Jan 6, 2024

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.

Ajnasz added a commit to Ajnasz/telescope-runcmd.nvim that referenced this issue Jan 6, 2024
Telescope sets the cursor column to +1, so after the telescope window
closed, the cursor is off by one which makes some operations to insert
to the wrong position.
To fix, restore the position to the original

Related issue:
nvim-telescope/telescope.nvim#2849
@jamestrew jamestrew linked a pull request Jan 6, 2024 that will close this issue
7 tasks
@jamestrew
Copy link
Contributor

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

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

@Ajnasz
Copy link
Author

Ajnasz commented Jan 7, 2024

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

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

@Ajnasz
Copy link
Author

Ajnasz commented Jan 8, 2024

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

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

seflue commented May 20, 2024

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

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

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.

3 participants