refactor(jump): only do extra search for target if there was no jump

[abeldekat]

Details:

Instead of always checking if target exists in text, only do so if there was no jump.
Additional benefit: The code using the cursor to check if a jump occurred can be replaced by a more direct check

• I have read CONTRIBUTING.md
• I have read CODE_OF_CONDUCT.md

[abeldekat]

@echasnovski,

The solution applied to solve issue #688 in February 2024 had a side-effect regarding highlighting.

Relevant part of diff:

@@ -364,6 +365,10 @@ H.make_expr_jump = function(backward, till)
     if target == nil then return '<Esc>' end
     H.update_state(target, backward, till, vim.v.count1)

+    vim.schedule(function()
+      if H.cache.has_changed_cursor then return end
+      vim.cmd('undo' .. (vim.fn.has('nvim-0.8') == 1 and '!' or ''))
+    end)
     return 'v<Cmd>lua MiniJump.jump()<CR>'

Whilst the scheduled undo command restored the character, it also prevented the highlighting.

Scenario with file containing a single word:

mark

Using its previous commit '1d49300d50a2'

• Open file, position cursor on the 'a'
• Type dfm
• Notice that the 'a' disappears, cursor is on 'r'
• Notice that 'm' is highlighted

Using the fix, commit '4f69339'

• Open file, position cursor on the 'a'
• Type dfm
• Notice that text is not modified, as expected
• Notice that m is not highlighted
• Type .
• Notice that 'm' is highlighted, as dot-repeat has no scheduled undo

To restore the original highlighting:

diff --git a/lua/mini/jump.lua b/lua/mini/jump.lua
index 234510b9..7bf50dd6 100644
--- a/lua/mini/jump.lua
+++ b/lua/mini/jump.lua
@@ -239,7 +239,8 @@ MiniJump.jump = function(target, backward, till, n_times)
   -- Ensure to undo "consuming a character" effect if there is no target found
   -- Do it here to also act on dot-repeat
   if has_jumped or not is_expr then return end
-  vim.schedule(function() vim.cmd('undo!') end)
+  -- vim.schedule(function() vim.cmd('undo!') end)
+  vim.api.nvim_create_autocmd('ModeChanged', { once = true, callback = function() vim.cmd('un
do!') end })
 end

 --- Make smart jump

I think it also is more precise to use the ModeChanged event.

[echasnovski]

Thanks for the PR!

I'll take a closer look a bit later (probably next week), as I can not quickly see if the new approach is optimal. Should be, as tests pass and it is negative diff 💪

To restore the original highlighting:
...
I think it also is more precise to use the ModeChanged event.

I don't really understand why ModeChanged event is better or even why it works. Initial reaction here is that executing the undo! command without vim.schedule might be dangerous due to how Neovim works (:h textlock, :h :map-expression, and similar things).

[abeldekat]

I don't really understand why ModeChanged event is better or even why it works. Initial reaction here is that executing the undo! command without vim.schedule might be dangerous due to how Neovim works (:h textlock, :h :map-expression, and similar things).

Me neither. I do think that the 'undo!' is the culprit here. If the command is deferred instead of scheduled, the highlighting is removed when the defer activates.

In addition to 'ModeChanged', 'CursorMoved' also works.

I also tried with 'nvim_feedkeys' and the escape character and did not succeed. In flash.nvim I found a snippet that does work. The diff with that code applied:

diff --git a/lua/mini/jump.lua b/lua/mini/jump.lua
index 234510b9..8fc8cb0e 100644
--- a/lua/mini/jump.lua
+++ b/lua/mini/jump.lua
@@ -239,7 +239,11 @@ MiniJump.jump = function(target, backward, till, n_times)
   -- Ensure to undo "consuming a character" effect if there is no target found
   -- Do it here to also act on dot-repeat
   if has_jumped or not is_expr then return end
-  vim.schedule(function() vim.cmd('undo!') end)
+
+  -- vim.schedule(function() vim.cmd('undo!') end)
+  local t = function(str) return vim.api.nvim_replace_termcodes(str, true, true, true) end
+  vim.api.nvim_feedkeys(t('<C-\\><C-n>'), 'nx', false)
+  vim.api.nvim_feedkeys(t('<esc>'), 'n', false)
 end

 --- Make smart _jump_

The disadvantage of that code is that test "can be dot-repeated if did not jump at first" breaks. I think one can argue that dot-repeating an operation that did not occur lacks intuitiveness. It's not supported by builtin dot-repeat. Surprisingly, flash is able to dot-repeat when initially the jump was not possible. Flash is not using 'expr' mappings though...

[echasnovski]

I also tried with 'nvim_feedkeys' and the escape character and did not succeed. In flash.nvim I found a snippet that does work. The diff with that code applied:

I would rather avoid using feedkeys() whenever possible. It proved to cause problems over time.

The disadvantage of that code is that test "can be dot-repeated if did not jump at first" breaks. I think one can argue that dot-repeating an operation that did not occur lacks intuitiveness. It's not supported by builtin dot-repeat.

Yeah, indeed built-in dot-repeat does not work that way. The current behavior still makes more sense to me as it allows a smoother repeated actions like: dtx -> switch buffer -> . -> etc.