[vim/vim] 'autocompletedelay' interferes with CTRL-G U (PR #20666)
zeertzjq
Problem: 'autocompletedelay' interferes with CTRL-G U (after 9.2.0739).
Solution: Restore the flag for CTRL-G U.
related: #8937
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/20666
Commit Summary
- 40861f5 'autocompletedelay' interferes with CTRL-G U
File Changes
(2 files)
- M src/edit.c (3)
- M src/testdir/test_ins_complete.vim (24)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20666)Oh, thanks for following up on this.
related #20598
I had Claude conduct a comprehensive audit to check for any other regressions. Here are the results.
(I haven't fully grasped everything yet myself...)
autocompletedelay (patch 9.2.0739) regression audit
Scope: regressions introduced by the async 'autocompletedelay' model
(commit 8ce43ea) that are NOT covered by the open PR #20666 (which fixes
i_CTRL-G_U and i_CTRL-K).
Root cause shared by most findings: the K_COMPLETE_DELAY pseudo-key is injected
by ui.c inchar_loop() guarded only by ins_compl_autocomplete_pending() && p_acl > 0 (plus maxlen >= 3 && !typebuf_changed). It lacks the gating that
trigger_cursorhold() (autocmd.c:1930-1945) applies to K_CURSORHOLD:
reg_recording == 0, typebuf.tb_len == 0, !ins_compl_active(), and
get_real_state() == MODE_NORMAL_BUSY || (state & MODE_INSERT). In addition the
arming flag compl_autocomplete_pending (insexpand.c:208) is cleared in only
three spots (edit.c:1174, edit.c:1431, and PR#20666's top-of-loop clear), so
Insert-mode exits that bypass them leave it armed.
1. Pending flag leaks out of Insert mode (HIGH)
stop_insert_mode exits jump to doESCkey via goto at edit.c:466-470
(pre-loop) and edit.c:651-673 (after vgetc). Both bypass every clear site;
ins_compl_prep(ESC) at edit.c:672 does not clear the flag either. If a
printable char armed the delay and then an async source ends Insert mode within
the delay window (a timer/channel callback or :stopinsert from a
TextChangedI/InsertCharPre autocmd or server command), the flag stays armed in
Normal mode. ui.c then injects K_COMPLETE_DELAY every 'autocompletedelay' ms;
normal.c has no K_COMPLETE_DELAY case, so the editor never goes idle and a
pending count/operator can be disrupted until Insert mode is re-entered.
Note: a plain ESC in the delay window is a real key read at the top of the loop,
so PR#20666's clear covers it. Only the stop_insert_mode (async) path leaks.
Fix: clear the pending flag on Insert exit (inside ins_compl_prep(ESC), or
before each goto doESCkey), and gate the ui.c injection on Insert mode.
2. K_COMPLETE_DELAY recorded into macros / scriptout (MEDIUM)
trigger_cursorhold() refuses to inject K_CURSORHOLD while reg_recording != 0,
precisely so the pseudo-key never lands in a recorded register. The ui.c
K_COMPLETE_DELAY injection (ui.c:337-356) has no such guard, and KE_COMPLETE_DELAY
is absent from any recording filter, so it is recorded by gotchars() during
q{reg} (and written to scriptout). Replaying such a macro then feeds a stray
K_COMPLETE_DELAY. The old synchronous busy-wait never produced a key, so this is
new.
Fix: add reg_recording == 0 (and typebuf.tb_len == 0) to the injection guard.
3. ui.c injection lacks the trigger_cursorhold gating (root, MEDIUM)
Beyond 1 and 2, the missing !ins_compl_active() and mode checks mean
K_COMPLETE_DELAY can be injected into a nested blocking input read driven from an
Insert-mode autocommand (e.g. input()/getchar() called from TextChangedI fired by
ins_redraw at edit.c:585, before the flag is cleared), and while completion is
already active. Gating the injection to mirror trigger_cursorhold() fixes 1's
secondary damage, 2, this, and the terminal/balloon cases below in one place.
4. Popup deferred by an extra ~'updatetime' when acl > ut (MEDIUM/LOW)
elapsed_time/start_tv are local to each inchar_loop() call. With
'autocompletedelay' > 'updatetime' and a CursorHoldI autocmd present, the first
call waits MIN(acl,ut)=ut and returns K_CURSORHOLD; the next call restarts the
clock and waits again, so the popup appears at roughly ut+acl instead of acl.
Worth confirming against intended timing.
5. K_COMPLETE_DELAY resets did_cursorhold (LOW, contested)
edit.c:1537-1543 sets did_cursorhold = FALSE for any key except K_CURSORHOLD;
K_COMPLETE_DELAY is not excluded. Combined with did_start_blocking being local to
inchar_loop, this may let CursorHoldI fire a second time in one idle period with
no intervening keypress, which contradicts the "not triggered again until a key
is pressed" spec. Verifiers split on reachability; needs a decision.
6. balloonexpr balloon dismissed by K_COMPLETE_DELAY (LOW)
getchar.c:2178 removes the terminal 'balloonexpr' balloon for any key except
K_MOUSEMOVE/K_IGNORE/K_CURSORHOLD; K_COMPLETE_DELAY is not excluded, so an armed
delay dismisses an open balloon. Add K_COMPLETE_DELAY to the exclusion.
7. terminal.c send-key switch has no K_COMPLETE_DELAY case (LOW, depends on 1)
terminal.c:1852 returns 0 for K_CURSORHOLD; there is no K_COMPLETE_DELAY case, so
a leaked pseudo-key (only reachable via finding 1) would be sent to the job as a
garbage unichar. Mooted if 1 and 3 are fixed.
Recommended fix (covers 1,2,3,6,7 at once)
A. Gate the ui.c K_COMPLETE_DELAY injection like trigger_cursorhold():
require Insert mode, reg_recording == 0, typebuf.tb_len == 0,
!ins_compl_active().
B. Clear compl_autocomplete_pending on Insert-mode exit (ins_compl_prep(ESC) is
the most robust single point, covering both stop_insert_mode gotos).
Findings 4 and 5 are independent and need a maintainer call on intended timing /
CursorHoldI semantics.
@zeertzjq May I separately publicize the measures mentioned above?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
zeertzjq
zeertzjq left a comment (vim/vim#20666)I noticed some of these problem after creating this PR, but I don't have enough time to fix them. Feel free to include this PR in a bigger one.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20666)Understood. I will do so. Thank you.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#20666)I created and submitted PR #20669, and included the following Note in that PR:
Note: this PR does not include the changes from #20666. Those fix separate
regressions from the same patch (i_CTRL-G_U and i_CTRL-K) and are not covered
here, so #20666 still needs to be merged separately
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.