Hmm, it seems that the current implementation will make 'autocompletedelay' affect the triggering delay of CursorHold.

Hmm, it seems that the current implementation will make 'autocompletedelay' affect the triggering delay of CursorHold.

Good catch — you're right. In the current PoC the input-wait timeout is
overwritten with p_acl whenever an autocomplete is pending, so while waiting
to trigger completion the CursorHold (p_ut) timing gets shadowed by
'autocompletedelay'. The two timers should coexist: wait for the sooner of
the two, pick the event by whichever deadline was reached, and not block
indefinitely while an autocomplete is still pending. I'll fix that. Thanks.

@girishji as the owner of the 'autocomplete' / 'autocompletedelay' feature, your review would be very welcome here.

The PR makes 'autocompletedelay' non-blocking: instead of the busy-wait in ins_complete(), the popup is deferred via an input-wait timeout (K_COMPLETE_DELAY) modeled on CursorHoldI, so typing stays responsive and the Insert-mode autocommands fire normally during the delay.

Two things I'd especially like your opinion on:

1. The overall approach — deferring the whole ins_complete() call to the idle timeout rather than only show_pum(). This also means matches are computed once after the delay instead of on every keystroke.

2. The still-open item, which is why CI is red. Three screendump tests fail — Test_autocompletedelay, Test_autocompletedelay_longest_preinsert, and Test_fuzzy_select_item_when_acl (they fail on every full-suite job: Linux huge / ASan / macOS). The reason is the new timing: now that the popup is deferred to the idle timeout, during the delay the cursor sits at a slightly different column and the popup isn't on screen at the moment these dumps were captured, so the committed dumps (which encode the old synchronous behavior) no longer match. There are no spurious characters or crashes — it's purely the new deferred-popup display.

   I've deliberately left the dumps untouched. If you agree the new behavior is correct, I'll regenerate the three dumps to match and CI goes green. Could you confirm whether the new preinsert / longest behavior is what it should be, and whether replacing the dumps is the right call?

Thanks!

@h-east can you capture a screencast to show how cursor behavior is different with your changes?

Wasn't familiar with the code path, noticed that keyboard input would interrupt pum show and ins_compl_delete ins_compl_restart undo the ins_complete. But it seems like I don't see this part in your change, maybe I misunderstand sth, or we deliberately drop this?

Ok, a stupid question. ins_compl_restart and ins_compl_delete is used to undo the thing we made in ins_complete . And since we reuse wait_func in in_char_loop , we would never get interrupt in ins_complete so just drop this.

@girishji
Sorry, my initial verification was insufficient. I am currently re-testing everything.
In the meantime, the old versions of the four dump files were wrong, so I've updated them to the correct ones. Please see the commit message for more details.

@Mrpaoo
I'm actually looking into the CI test errors right now and just encountered that issue. Currently working on it. Thanks for reporting!

While investigating, I found that master has a pre-existing bug: the ruler
does not correctly show the cursor position while the completion popup is
displayed. Several of the test_ins_complete screendumps will need to be
updated.

The CI failures came down to two separate things, and I've updated the
branch (squashed into a single commit) accordingly.

1. A timing issue in the test itself. Test_autocompletedelay opened the
menu with CTRL-N right after sleep-ing for exactly 'autocompletedelay'
milliseconds, which raced with the now-deferred popup: depending on
scheduling, the key could arrive before the popup was shown. The test now
waits a little longer than the delay before sending the key. This was a test
assumption that only held under the old synchronous (busy-wait) behavior.

2. The ruler column shown while the completion popup is up.

On (2): the dumps that are supposed to check the state during the delay were
also misleading. VerifyScreenDump retries until the screen matches the
committed dump; because the committed ruler column didn't match, it kept
retrying until the delay elapsed and the popup appeared — so a "during the
delay" dump actually captured the post-delay popup. In other words the
popup was never shown early; the retry simply outlasted the delay. I split
these into proper "during the delay" (no popup) and "after the delay" (popup)
dumps, so the during-delay ones now verify that the popup is correctly not
shown yet.

After that, the ruler shows the cursor column in all the popup dumps except
two: Test_autocompletedelay_6 (Backspace) and Test_fuzzy_autocompletedelay_3
(typing into an already-open menu). Both go through ins_compl_new_leader(),
which repositions the popup by temporarily moving the cursor to the completion
column (ins_compl_show_pum()), leaving the ruler at that column. So the
remaining discrepancy is confined to the new_leader path.

I'd keep this PR focused on the async change and fix that as a separate patch —
ideally by positioning the popup menu without moving w_cursor (let the
drawing side compute its placement), which is the root cause and would also
make the other dumps robust rather than timing-dependent. The two dumps in
this PR reflect the current (deterministic) compl_col behavior.

@girishji does that sound reasonable to you?

Looks good, but there are many moving parts. I suggest you use this delay feature in your workflow for a few days to make sure there are no issues. Also, I think you should prioritize fixing Test_autocompletedelay_6 (Backspace) and Test_fuzzy_autocompletedelay_3 as part of this commit.

Squashed into a single commit and updated the commit message and PR description to reflect the latest changes.

Also, I think you should prioritize fixing Test_autocompletedelay_6 (Backspace) and Test_fuzzy_autocompletedelay_3 as part of this commit.

The above-mentioned fix has also been incorporated into this PR. For further details regarding this change, please see the commit message or PR description.

While testing with 'autocompletedelay' set, I found a behavior change for
Backspace when the popup menu is already shown:

$ vim --clean +"set ac acl=500" +"call setline(1, ['foobar foobaz foobiz', ''])" +"call feedkeys('GCfoob', 't')"

• master: typing foob shows the popup; <BS> then updates it immediately.
• this PR: the same <BS> hides the popup and re-shows it only after
  'autocompletedelay' has elapsed.

It comes from ins_compl_new_leader(): with 'autocompletedelay' it now
re-arms the delay on a leader change, instead of calling show_pum() right
away as master does.

Two ways to look at it: re-arming is consistent (every leader change is
debounced), but when the popup is already visible, narrowing it with <BS>
arguably reads better if it updates immediately the way master does — hiding
and waiting for the delay feels sluggish for an already-open menu.

One possibility would be to show the popup immediately when it is already
visible, and only re-arm the delay while it is still hidden (pending).
Do you have a preference here?

(Added:)
Ah, it’s the same for regular key inputs when the popup is shown.

While testing with 'autocompletedelay' set, I found a behavior change for
Backspace when the popup menu is already shown:

Ah, it’s the same for regular key inputs when the popup is shown.

I will fix it to follow the master's behavior.

--> Done.

@girishji @chrisbra A heads-up on the ruler fix we agreed to include here.

I have split it out into a separate, master-based PR: #20626.

The reason is that it turned out to be a pre-existing bug that is independent of
'autocompletedelay': the ruler shows the completion-start column instead of
the real cursor column whenever the popup menu is visible, including with a
status line and for normal completion - not only in the delayed case. It also
needed one more change than the _6/fuzzy_3 fix (the status line was not
redrawn promptly), and fixing it deterministically removes the long-standing
local-vs-CI mismatch in the completion screendumps. Keeping it as its own change
makes both PRs cleaner and lets the ruler fix be reviewed on its own.

So this still addresses the ruler problem you asked about - just in a dedicated
PR rather than bundled here.

@chrisbra - the merge order matters here: #20626 should go in first (I've also
noted this at the top of the PR description). After it lands I'll rebase this PR
on top and drop the duplicated pum_display() change, leaving only the
'autocompletedelay' feature.

I rebased onto master.