folke / flash.nvim

Navigate your code with search labels, enhanced character motions and Treesitter integration
Apache License 2.0
2.47k stars 33 forks source link

bug: Mapping that touches `w` causes remote flash (with `y` and `c`) followed by inner-word to fail #165

Closed linduxed closed 1 year ago

linduxed commented 1 year ago

Did you check docs and existing issues?

Neovim version (nvim -v)

NVIM v0.9.1

Operating system/version

Linux

Describe the bug

The "remote" feature seems to break somewhat if one does onoremap = w, where the = could seemingly be any key (I've tried with a bunch, it seems to be the same result). The reason for this mapping is that I've moved a bunch of keys around to account for me using Colemak. This means that I'm not just doing an onoremap for w, but also nnoremap and xnoremap as well.

Let's presume one applies the onoremap, then initiates a change with c. From here, one can initiate the "remote flash" (with whatever mapping one has set for that), but here's where a problem appears: running inner-word with iw fails. While one would expect that one can do the change, then come back to the original cursor position, instead one has the entire action cancelled.

The effect is that one can jump to the remote position, but then one can't run iw. What happens is that one simply gets the entire remote flash procedure cancelled.

My theory (without having dug around in the code) is that when one initiates the change with c, and then initiates the remote flash, one kind of gets out of o-mode (or something like that), which results in w needing to be set to the usual w, otherwise the entire flash gets cancelled.

Mapping keys to w seems to break c and y, but d seems to work (for some reason).

Steps To Reproduce

  1. nnoremap = w
  2. Normal-mode c
  3. Initiate remote flash
  4. Try to do iw

Expected Behavior

One gets to do the change and get the cursor back in the orginal place. Instead, one gets back the cursor, but the changing was not initiated.

Repro

-- DO NOT change the paths and don't remove the colorscheme
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.loop.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 = {
  "folke/tokyonight.nvim",
  {
    "folke/flash.nvim",
    event = "VeryLazy",
    opts = {},
    keys = {
      { "gr", mode = { "o" }, function() require("flash").remote() end, },
    },
  },
  -- add any other plugins here
}
require("lazy").setup(plugins, {
  root = root .. "/plugins",
})

vim.cmd.colorscheme("tokyonight")
-- add anything else here

-- the following mappings make both flash remote yanking and changing broken.
vim.keymap.set({ "n", "o", "x" }, "y", "w")
vim.keymap.set({ "n", "o", "x" }, "c", "y")
vim.keymap.set({ "n", "o", "x" }, "w", "c")
folke commented 1 year ago

Can you please describe step by step how to reproduce, what keys you press and what action you intend to perform? This is all pretty confusing.

linduxed commented 1 year ago

Sorry for not being clear enough!

The intention is to simply do a remote (require("flash").remote()) change (c), nothing more.

Procedure

If I was to express that it Neovim motions:

Let's presume that I have no custom mappings (that Colemak stuff that I described above), and then use the default configuration for flash.nvim (as listed in the README, i.e. remote flash is set to r in o-mode).

This means that I would want to do the following:

  1. c (change)
  2. r (initiate remote flash)
  3. Type letters for the flash jump. Confirm a label.
  4. (In the new cursor position) iw. Intention: changing (c) the word that the cursor is on.
  5. Typing something.
  6. <esc>

The issue

In the repro, at the very end of the script, there are three mappings added, each applied to modes n, o, and x.

With these mappings added, the procedure above fails on step no. 4, i.e. when one tries to do iw. What happens instead of letting the user type (step no. 5 in the procedure above), one gets the same result as hitting <esc>, i.e. cancelling the change, and then returning to the original position.

It might be that one could make the repro even more minimal with some of the modes removed for some of the mappings, but the interactions are a bit unclear.

Minor detail about the repro

In the originally submitted version of the repro script, I had included a mapping for closing Neovim with <Space>q. This was there for my own convenience during repro testing, and was supposed to have been removed prior to submitting this issue.

linduxed commented 1 year ago

As an extra detail: with the mappings in the repro, doing yank (mapped to c in the repro) fails right after one chooses a label, i.e. one does not even get to do iw before one is kicked out to the original cursor position.

max397574 commented 1 year ago

With these mappings added, the procedure above fails on step no. 4, i.e. when one tries to do iw. What happens instead letting the user type (step no. 5 in the procedure above), one gets the same result as hitting , i.e. cancelling the change, and then returning to the original position.

Of course you can't type anything. Because you mapped c to y which just takes a text-object or motion and then returns or am I missunderstanding something here?

edit: a few things about your mappings afaik when something is mapped to a key the key that is being mapped to doesn't care at all. There should be absolutely no change in behavior. the two operator pending mappings w and iw are independent

folke commented 1 year ago

Not home right now, but indeed the only way I could see this fail is if vim.v.operator would be wrong by using these keymaps. If so, that would be a Neovim bug. Will look into it later today

folke commented 1 year ago

My fix is a bit ugly, but it works.

linduxed commented 1 year ago

Thank you for your work, this does indeed fix the issue!

linduxed commented 1 year ago

Update: I've noticed that the fix, while it has made it possible to do the remote actions (yank, change, etc.), if one does a remote yank of a word, the mapping on y gets unmapped (resulting in :nmap y showing "No mapping found").

Any ideas why this could be?