Skip to content

Fix: multiple bug fixes and test suite repairs - #30

Closed
ImproAnt wants to merge 4 commits into
xiaolai:mainfrom
ImproAnt:main
Closed

Fix: multiple bug fixes and test suite repairs#30
ImproAnt wants to merge 4 commits into
xiaolai:mainfrom
ImproAnt:main

Conversation

@ImproAnt

Copy link
Copy Markdown
Contributor

Summary

  • Fix zombie tab on file read failure, search wrap-around direction, and stale match count — prevents orphaned tabs when file reads fail, fixes search direction on wrap-around, and corrects stale match count display
  • Fix curly quote auto-pair not working with IME input — resolves auto-pair handler not triggering for IME-composed characters
  • Fix auto-save all tabs, sequential quit, and stale content detection — ensures all dirty tabs are saved, quit proceeds sequentially, and external file changes are detected correctly
  • Fix 16 pre-existing test failures in test setup — adds localStorage mock for zustand persist middleware, mock modules and vitest aliases for @panzoom/panzoom and tauri-pty so all 231 test files (3981 tests) pass

Test plan

  • All 231 test files pass (3981 tests, 0 failures)
  • Verify auto-save works across multiple tabs
  • Verify search wrap-around direction is correct
  • Verify curly quote auto-pair works with IME input
  • Verify file read failure doesn't leave zombie tabs

🤖 Generated with Claude Code

chriszhou and others added 4 commits February 11, 2026 08:30
Three high-impact bugs found during full codebase audit:

1. Auto-save only saved the active tab — background dirty tabs were never
   auto-saved, risking data loss on crash. Now iterates all tabs with
   per-tab debounce tracking.

2. Quit broadcast caused race condition — all windows received quit events
   simultaneously, causing overlapping save dialogs. Now processes windows
   sequentially via an ordered queue with acknowledge/cancel protocol.

3. File watcher blind spots on tab switch — in non-workspace mode, the
   watcher only monitors the active tab's directory. Added a hook that
   checks file freshness from disk when switching tabs.

Also clears LAST_EMITTED debounce state in stop_all_watchers to prevent
stale entries from suppressing future events.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Relax the IME grace period guard in handleTextInput (only block during
active composition, not the 50ms grace window) so quick IME-produced
punctuation like " triggers auto-pair. Add compositionend handler that
applies auto-pair/skip-over logic for composed single characters after
ProseMirror commits the text.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nd stale match count

- Clean up zombie tab and show error toast when readTextFile fails in openFileInNewTab
- Track find direction explicitly in searchStore to fix wrap-around navigating wrong way
- Poll for document changes in source editor search to keep match count fresh while editing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add localStorage mock for zustand persist middleware, mock module files
and vitest aliases for @panzoom/panzoom and tauri-pty so Vite can
resolve imports before vi.mock intercepts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@xiaolai

xiaolai commented Feb 11, 2026

Copy link
Copy Markdown
Owner

Thanks for the comprehensive work here! There are some genuinely good fixes in this PR, but a few concerns prevent us from merging as-is.

What looks good

  • Zombie tab fix — Clean and correct. Closing the speculatively-created tab on readTextFile failure is the right approach.
  • Search wrap-around direction — Solid fix. Tracking direction explicitly via _lastFindDirection instead of inferring from index comparison is correct.
  • Stale match count polling — Pragmatic 300ms poll approach works fine.
  • Watcher debounce clearing — Clean bugfix, stop_all_watchers() should clear LAST_EMITTED.
  • Test infrastructure — The mocks for panzoom, tauri-pty, and localStorage are reasonable.

Concerns

1. Cargo.lock bumps Tauri 2.9.5 → 2.10.2 (and wry 0.53 → 0.54)

This is a significant runtime change — the webview layer itself changed minor versions. This kind of dependency bump should be a separate, isolated PR so we can test for visual and behavioral regressions independently. Bundling it with functional changes makes it hard to bisect if something breaks.

2. IME auto-pair compositionend handler — fragile timing

The requestAnimationFrame approach in the compositionend handler assumes ProseMirror commits the composition synchronously before the next frame. If ProseMirror's composition handling is also deferred, the callback could see stale state. The interaction between handleTextInput and compositionend is subtle — both can now fire for the same character (the grace period change), relying on guards to prevent double-pairing. This needs thorough manual testing with actual CJK IME input (Chinese pinyin, Japanese kana, Korean) before we can merge.

3. useTabSwitchFileCheck.ts duplicates logic from useExternalFileChanges

The new hook duplicates significant logic: resolveExternalChangeAction switch, save dialog, reload logic. This creates a maintenance burden — any change to external change handling needs updating in two places. We'd prefer extracting shared logic into a common utility.

4. File read on every tab switch

Every tab switch triggers a readTextFile call. For large files or slow disks, this could introduce perceptible latency. There's also a narrow race with pending saves — if a save is in-progress when switching, a partial file read could trigger a spurious "file changed" prompt.

5. Sequential quit — minor double-emit concern

Both acknowledge_quit_window and handle_window_destroyed call process_next_quit_target, which could emit app:quit-requested twice to the next window. The isClosingRef guard mitigates this, but it's worth adding a guard in process_next_quit_target itself.

Suggestion

Would you consider splitting this into smaller PRs? The individual fixes are valuable but mixing them with the Tauri version bump and the IME timing changes makes it risky to merge as one unit. For example:

  1. PR A: Zombie tab fix + search wrap-around + stale count + watcher debounce (low risk, merge quickly)
  2. PR B: Cargo.lock / Tauri bump (isolated, easy to test)
  3. PR C: Sequential quit + auto-save all tabs
  4. PR D: IME auto-pair (needs dedicated CJK testing)
  5. PR E: Tab switch file check (after extracting shared logic with useExternalFileChanges)

Thanks again for the thorough work — we'd love to get the good parts in!

@xiaolai xiaolai closed this Feb 11, 2026
xiaolai added a commit that referenced this pull request Feb 19, 2026
Issues fixed:
- #6: Add StatusBar warning when auto-save paused (file missing)
- #15: Use Documents folder instead of Home for default save location
- #19: Fix race condition in recent files menu by storing snapshot in Rust
- #30: Fix dock icon drop when no document windows exist
- #35: Show toast notification when file auto-reloads
- #37: Add "Restore to Disk" context menu for missing files

Additional improvements:
- Add toast on pinned tab close attempt (#7)
- Add toast on save failure (#5/#14)
- Add toasts for drag-drop failures (#25, #26, #27)
- Improve cold start file open reliability (#9, #34)
- Fix no-window menu operations (#17, #18, #21)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants