Audit follow-ups: backup-suffix parsing, terminal-cleanup gating, docs#12
Open
chiro-hiro wants to merge 3 commits into
Open
Audit follow-ups: backup-suffix parsing, terminal-cleanup gating, docs#12chiro-hiro wants to merge 3 commits into
chiro-hiro wants to merge 3 commits into
Conversation
Same-second re-installs append a .<counter> to the .bak.<epoch> name to avoid collisions, but the restore path only parsed the bare epoch — so on a same-second collision it compared the whole "<epoch>.<counter>" string against the integer regex, rejected it, and could restore the wrong (or no) backup. Split the suffix into <epoch> and <counter>, validate both as integers, and break ties on the counter so uninstall restores the truly newest backup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
VimLeavePre fires too late to pre-empt the E947 "job still running" check
that blocks :qall when a terminal is open. Switch back to QuitPre, but
guard it: count non-terminal ("ordinary") windows and only wipe terminal
jobs when the window being quit is the last one. This threads between both
failure modes — no spurious kills on a single-window or aborted :q, and no
E947 on a real :qall exit. Update the CLAUDE.md Terminal note to match.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Autocomplete section illustrated triggers as ".", "::", "->", but the engine matches a single character against b:ivim_complete_triggers and no ftplugin registers multi-char strings — "::" was deliberately removed because it froze Rust/Lua/C++ completion. Replace the list with the actual single-char union across ftplugins: . > : < / $ and space. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up audit fixes on top of the merged #1. Three small, independent changes.
Changes
Installer backup-suffix parsing (
install.sh,get-ivim.sh) —find_latest_backup()now parses the<epoch>.<counter>suffix introduced for same-second collision avoidance. Before this, a same-second collision produced a.bak.<epoch>.<counter>name that failed the bare-integer check, so uninstall could restore the wrong backup (or none). Both components are now validated as integers and ties break on the counter.Terminal cleanup gating (
plugin/keymaps.vim+ matching CLAUDE.md note) — move cleanup offVimLeavePre(which fires too late to pre-empt theE947"job still running" check on:qall) back toQuitPre, but gated on the last ordinary (non-terminal) window. Result: no spurious terminal kills on a single-window or aborted:q, and noE947on a real:qallexit.Docs (
CLAUDE.md) — correct the autocomplete trigger-char list to the real single-char union (.>:</$and space) and drop the phantom::/->examples. Addresses the CLAUDE.md item in Documentation drift: start-screen “any key”, CLAUDE.md triggers, README trigger list & structure tree #11.Verification
bash -nclean on both installerskeymaps.vimsources without error; autocmd is nowQuitPre → s:CloseTerminalsOnExitNot addressed here (kept open)
<leader>x(:xall) leaves terminal job running → E948; cleanup only fires on QuitPre #2 (<leader>x/:xall→E948)::xallbypassesQuitPreentirely, so the last-window gate never runs for save-and-quit-all. The CLAUDE.md terminal note committed here ("auto-closes when Vim exits") is accurate for:q/:qallbut overstates the:xallpath —<leader>x(:xall) leaves terminal job running → E948; cleanup only fires on QuitPre #2 tracks that gap.<leader>x(:xall) leaves terminal job running → E948; cleanup only fires on QuitPre #2–Documentation drift: start-screen “any key”, CLAUDE.md triggers, README trigger list & structure tree #11 (filed from a multi-agent source review).🤖 Generated with Claude Code