-
Notifications
You must be signed in to change notification settings - Fork 162
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
feat: allow repeating the last completion with .
#870
base: main
Are you sure you want to change the base?
Conversation
I'd be open to merging this early and making it opt-in: |
Nice! I just rebased and accounted for To be honest, I haven't had the time to test any more cases than the ones in the demo. I think automatic parenthesis insertion could still be supported (might not work right now) - snippets might be too difficult (I don't know how they are implemented). I'm leaving for a mini holiday today for a day or two, but if you want to merge this in sooner, it might work fine as an opt-in feature. In general, I have found that merging opt-in features often tends to speed things up quite a bit 👍🏻 |
Just quickly tested a case where parentheses are inserted - it seems to have an issue with positioning the cursor (although repeating does work surprisingly) bug.mov |
I think I found out the cause. It's outlined here (neovim/neovim#16571 (comment)), but I'll give the short version: if I understand correctly, using I think I was able to fix the positioning bug by simply waiting until the keys have been scheduled (I think it runs later in the event loop which works), but I had to convert some code to use position.movAs you can see, the cursor goes too far to the right now. I can dig into why that happens later on. But I also wanted to hear your thoughts on using |
I also noticed that in my ripgrep plugin, blink now calls the source for every new character. This causes a lot of ripgrep searches, while previously the source was only called once. 😦 Maybe my async implementation has some issues? |
This patch seems works for me blink.cmp git:(main) ✗ git --no-pager diff HEAD 0 [04:37:51]
diff --git a/lua/blink/cmp/completion/accept/init.lua b/lua/blink/cmp/completion/accept/init.lua
index c4041d3..3811de5 100644
--- a/lua/blink/cmp/completion/accept/init.lua
+++ b/lua/blink/cmp/completion/accept/init.lua
@@ -78,6 +78,14 @@ local function accept(ctx, item, callback)
-- OR Normal: Apply the text edit and move the cursor
else
table.insert(all_text_edits, item.textEdit)
+
+ local row = vim.api.nvim_win_get_cursor(0)[1]
+ local old_line = vim.api.nvim_get_current_line()
+ -- emulate builtin completion (dot repeat).
+ vim.fn.complete(item.textEdit.range.start.character + 1, { item.textEdit.newText })
+ -- restore old content
+ vim.api.nvim_buf_set_lines(0, row - 1, row, false, { old_line })
+
text_edits_lib.apply(all_text_edits)
-- TODO: should move the cursor only by the offset since text edit handles everything else?
ctx.set_cursor({ ctx.get_cursor()[1], item.textEdit.range.start.character + #item.textEdit.newText + offset
}) nvim-cmp has some heuristics to determine a good
The performance cost is 1ms ~ 4ms, the same as feedkey's cost in nvim-cmp. This is the relevant coc.nvim dot-repeat pr using I'm just giving some idea here, IMO nvim-cmp uses |
Whenever you feel this is ready for merging, mark it as ready to review and I'll add the bits to make it opt in. Thanks for all your work on this! |
Nice, I appreciate it! There's one major bug left - I think that should be fixed before merging: I created this demo to clarify what I meant. Each time the text flashes with the bright color, a new search is done (this is wrong). Incorrectinvocations.movI think I have broken the old behaviour of only searching once, and using the following keypresses as filters. That can be seen here (text flashes only once) Correct old behaviour 👍🏻invocations-working.mov |
@xzbdmw did you apply that patch on top of this PR, or did you apply it on top of the newest version (nightly blink)? Meaning, it that all that is needed for it to work? I could try it out. It looks much simpler, but I can't tell if it has some limitations or not. |
Yeah, it apply on top of main branch. (there should be some edge case) If you use auto_insert, this approach surprisingly triggers CursorMovedI, even if I set even_ignore = "all", I see I'm listing a newest path: blink.cmp git:(dot-repeat) git --no-pager diff HEAD~1 HEAD 0 [01:20:30]
diff --git a/lua/blink/cmp/lib/text_edits.lua b/lua/blink/cmp/lib/text_edits.lua
index 2ce76fe..e1bd6c6 100644
--- a/lua/blink/cmp/lib/text_edits.lua
+++ b/lua/blink/cmp/lib/text_edits.lua
@@ -7,7 +7,26 @@ local text_edits = {}
--- @param edits lsp.TextEdit[]
function text_edits.apply(edits)
local mode = context.get_mode()
- if mode == 'default' then return vim.lsp.util.apply_text_edits(edits, vim.api.nvim_get_current_buf(), 'utf-8') en
d
+ if mode == 'default' then
+ local edit = edits[1]
+ local row = vim.api.nvim_win_get_cursor(0)[1]
+ local old_line = vim.api.nvim_get_current_line()
+ vim.o.eventignore = 'all'
+ -- emulate builtin completion (dot repeat).
+ vim.fn.complete(edit.range.start.character + 1, { edit.newText })
+
+ local feedkeys = function(keymap, mode)
+ vim.api.nvim_feedkeys(vim.api.nvim_replace_termcodes(keymap, true, false, true), mode, false)
+ end
+ -- exit completion mode
+ feedkeys('<c-x><c-z>', 'in')
+ -- restore old content, use nvim_buf_set_text to preserve extmark.
+ vim.api.nvim_buf_set_text(0, row - 1, 0, row - 1, -1, { old_line })
+ vim.lsp.util.apply_text_edits(edits, vim.api.nvim_get_current_buf(), 'utf-8')
+ vim.o.eventignore = ''
+
+ return
+ end
assert(mode == 'cmdline', 'Unsupported mode for text edits: ' .. mode)
|
I tried that implementation. I like that it's so short and simpler to understand. It passed all my tests in blink-ripgrep, including the cases in my repeat demo. But a more complicated case seems to fail: multiple.movI noticed my implementation also fails to work in this case, so I will fix that next. |
Seems work for me (I have empty config) iShot_2025-01-08_03.50.59.mp4 |
I fixed the bug with many subsequent invocations, and that (basic) case works now many-repeats.movThere is one remaining bug that I know of: selecting other completion items now just inserts them instead of just highlighting them. I'm a bit stressed for time, but will take a look at this later. edit: also, the repeated invocations bug from earlier is not a bug after all. My bad on that one. 🙂 |
Spent a little bit more time on this, but was not able to fix the bug. scroll-inserts.mov@Saghen would you have some idea on what could be wrong? I looked into how events are handled in lua/blink/cmp/lib/buffer_events.lua, but I think no new events are handled by accident. Maybe it's possible that keeping track of the last inserted character is somehow messed up? |
We update the context on every P.S. Sorry I haven't been more active to help you with this, work has been busy lately |
No worries - a normal day in open source 🙂 I think you're right about the event stuff. I wasn't able to cleanly solve all of my test cases, but I found out not applying the preview for the selected completion fixes the bug in my previous message: diff --git a/lua/blink/cmp/completion/list.lua b/lua/blink/cmp/completion/list.lua
index eeae0fe..40853c5 100644
--- a/lua/blink/cmp/completion/list.lua
+++ b/lua/blink/cmp/completion/list.lua
@@ -225,6 +225,7 @@ function list.undo_preview()
end
function list.apply_preview(item)
+ if true then return end
-- undo the previous preview if it exists
list.undo_preview()
-- apply the new preview Maybe you have an idea what a clean way to do this could look like? |
It should be ignored because the |
Just found out that additional text edits don't work yet 😄 I can take a look at this problem in the coming days. oops.mov |
Yeah they should be applied via |
Yeah, applying the additional text edits before the actual (now repeatable) text edit seems to work great nice.movJust pushed this change. But this still needs applying the Details
diff --git a/lua/blink/cmp/lib/buffer_events.lua b/lua/blink/cmp/lib/buffer_events.lua
index dcca8b8..d2f7ee6 100644
--- a/lua/blink/cmp/lib/buffer_events.lua
+++ b/lua/blink/cmp/lib/buffer_events.lua
@@ -131,7 +131,8 @@ function buffer_events:suppress_events_for_callback(cb)
local is_insert_mode = vim.api.nvim_get_mode().mode == 'i'
- self.ignore_next_text_changed = changed_tick_after ~= changed_tick_before and is_insert_mode
+ -- self.ignore_next_text_changed = changed_tick_after ~= changed_tick_before and is_insert_mode
+ self.ignore_next_text_changed = true
-- HACK: the cursor may move from position (1, 1) to (1, 0) and back to (1, 1) during the callback
-- This will trigger a CursorMovedI event, but we can't detect it simply by checking the cursor position |
Without disabling weird.movMaybe you could take a look at it? The event handling logic looks a bit intricate to me right now. |
I spent a couple hours trying to get this working but wasn't successful. This adds more complexity than I was hoping for since making text edits async means downstream also needs to be async. It's odd though, since the feedkeys implementation doesn't seem like it needs to be async, it appears to be running synchronously. I'm toying around with a |
This is a bit work in progress, but could implement dot-repeat (
.
) for simple completions. All cases might not work yet.repeat-demo.mov
The algorithm is basically the same as in nvim-cmp. The way it works here is the following:
.
, dot). This register gets executed when the user presses the dot key later on. Note that neovim does not allow cleanly replacing text in a way that supports dot-repeat (nvim_buf_set_text in insert mode should update dot repeat neovim/neovim#19806 (comment))next steps
Solves #182