From 03173421f2f20e2c139976b2ba8bb09f6f9518bc Mon Sep 17 00:00:00 2001 From: Aditya Date: Mon, 29 Jun 2026 15:26:56 +0530 Subject: [PATCH 1/2] refactor: deduplicate shortcuts and manage timer completion timeouts --- AUDIT_LOG.md | 12 ++++++ CHANGELOG.md | 5 +++ src-tauri/src/commands/shortcuts.rs | 44 +++++++++------------- src/lib/editor/VariableScope.test.ts | 55 ++++++++++++++++++++++++++++ src/store/useTimerStore.ts | 28 +++++++++++--- 5 files changed, 113 insertions(+), 31 deletions(-) create mode 100644 src/lib/editor/VariableScope.test.ts diff --git a/AUDIT_LOG.md b/AUDIT_LOG.md index 350a68d..ee46b25 100644 --- a/AUDIT_LOG.md +++ b/AUDIT_LOG.md @@ -2,6 +2,18 @@ This log tracks all significant changes, updates, and versions in the PaperCache project. +## 2026-06-29 (Code Quality Refactor & Test Suite) +**Change:** refactor(shortcuts): extract helper to deduplicate global shortcut trigger logic; fix(timers): manage completion timeout lifecycle in store; test(editor): add comprehensive unit test suite for `VariableScope` + +**Details/Why:** +1. **Shortcut Deduplication**: Extracted `handle_shortcut_trigger` helper in `src-tauri/src/commands/shortcuts.rs` to replace 16 lines of identical duplicate code across `update_global_shortcut` and `resume_shortcuts`. +2. **Managed Timeout Lifecycle**: Replaced unmanaged 5-second `setTimeout` in `useTimerStore.ts` with a tracked Map of active timeouts aligned to `COMPLETED_TIMER_CLEANUP_MS` (10s), ensuring timers cleaned up early or removed explicitly do not trigger orphan state updates. +3. **VariableScope Unit Tests**: Created `src/lib/editor/VariableScope.test.ts` testing global/note scope merging and debounced regex mathematical expression parsing (`/var x = ...`) using fake timers. + +**Files changed:** `src-tauri/src/commands/shortcuts.rs`, `src/store/useTimerStore.ts`, `src/lib/editor/VariableScope.test.ts`, `AUDIT_LOG.md`, `CHANGELOG.md`. + +--- + ## 2026-06-29 (Code Quality Cleanup) **Change:** refactor: code quality cleanup — dead code, boilerplate, types, constants, AI comments; fix: address PR review findings — listener leak, type contracts, dead ref, stale guard, cfg scope, shortcut loop, timer constant diff --git a/CHANGELOG.md b/CHANGELOG.md index 52cff4a..44baa5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable, user-facing changes to PaperCache will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed +- **Code Quality & Test Reliability**: Refactored global shortcut registration to remove duplicate event handling logic in the backend. Improved countdown timer cleanup reliability by properly tracking and clearing async timeouts when timers complete or are removed. Added comprehensive unit tests for inline DSL variable evaluation (`VariableScope`). + ## [v0.5.6] - 2026-06-28 ### Added diff --git a/src-tauri/src/commands/shortcuts.rs b/src-tauri/src/commands/shortcuts.rs index 1e815b4..27aa51e 100644 --- a/src-tauri/src/commands/shortcuts.rs +++ b/src-tauri/src/commands/shortcuts.rs @@ -15,6 +15,22 @@ impl Default for GlobalShortcutState { } } +fn handle_shortcut_trigger(app: &AppHandle, action: &str) { + if action == "new-note" { + if let Some(window) = app.get_webview_window("main") { + if !window.is_visible().unwrap_or(false) { + let _ = window.show(); + let _ = window.set_focus(); + #[cfg(target_os = "macos")] + crate::macos::force_focus(); + } + } + } else { + crate::commands::system::toggle_window(app); + } + let _ = app.emit(&format!("trigger-{}", action), ()); +} + #[tauri::command] pub fn update_global_shortcut( app: AppHandle, @@ -39,19 +55,7 @@ pub fn update_global_shortcut( app.global_shortcut() .on_shortcut(shortcut, move |app, _shortcut, event| { if event.state() == ShortcutState::Pressed { - if action_clone == "new-note" { - if let Some(window) = app.get_webview_window("main") { - if !window.is_visible().unwrap_or(false) { - let _ = window.show(); - let _ = window.set_focus(); - #[cfg(target_os = "macos")] - crate::macos::force_focus(); - } - } - } else { - crate::commands::system::toggle_window(app); - } - let _ = app.emit(&format!("trigger-{}", action_clone), ()); + handle_shortcut_trigger(app, &action_clone); } }) .map_err(|e| format!("Failed to register shortcut: {}", e))?; @@ -84,19 +88,7 @@ pub fn resume_shortcuts(app: AppHandle) -> Result<(), String> { .global_shortcut() .on_shortcut(shortcut, move |app, _, event| { if event.state() == ShortcutState::Pressed { - if action_clone == "new-note" { - if let Some(window) = app.get_webview_window("main") { - if !window.is_visible().unwrap_or(false) { - let _ = window.show(); - let _ = window.set_focus(); - #[cfg(target_os = "macos")] - crate::macos::force_focus(); - } - } - } else { - crate::commands::system::toggle_window(app); - } - let _ = app.emit(&format!("trigger-{}", action_clone), ()); + handle_shortcut_trigger(app, &action_clone); } }); } diff --git a/src/lib/editor/VariableScope.test.ts b/src/lib/editor/VariableScope.test.ts new file mode 100644 index 0000000..a0d5dc9 --- /dev/null +++ b/src/lib/editor/VariableScope.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest' +import { VariableScope, getScope } from './VariableScope' +import { useVariableStore } from '../../store/useVariableStore' + +describe('VariableScope', () => { + beforeEach(() => { + vi.useFakeTimers() + useVariableStore.getState().setGlobals({}) + useVariableStore.getState().setNoteScope({}) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('merges global and note scopes in getScope', () => { + useVariableStore.getState().setGlobals({ globalA: 10, shared: 'global' }) + useVariableStore.getState().setNoteScope({ noteB: 20, shared: 'note' }) + + const scope = getScope() + expect(scope).toEqual({ + globalA: 10, + noteB: 20, + shared: 'note', + }) + }) + + it('parses mathematical expressions and updates note scope after debounce', () => { + const scopeMgr = new VariableScope() + const doc = '/var x = 10 + 5\n/var y = x * 2' + + scopeMgr.triggerScopeUpdate(doc, null) + + expect(useVariableStore.getState().getNoteScope()).toEqual({}) + + vi.advanceTimersByTime(300) + + expect(useVariableStore.getState().getNoteScope()).toEqual({ + x: 15, + y: 30, + }) + }) + + it('falls back to raw trimmed string if expression parsing fails', () => { + const scopeMgr = new VariableScope() + const doc = '/var greeting = Hello World' + + scopeMgr.triggerScopeUpdate(doc, null) + vi.advanceTimersByTime(300) + + expect(useVariableStore.getState().getNoteScope()).toEqual({ + greeting: 'Hello World', + }) + }) +}) diff --git a/src/store/useTimerStore.ts b/src/store/useTimerStore.ts index f7084d1..7ebb604 100644 --- a/src/store/useTimerStore.ts +++ b/src/store/useTimerStore.ts @@ -10,6 +10,16 @@ import { create } from 'zustand' const COMPLETED_TIMER_CLEANUP_MS = 10000 +const completionTimeouts = new Map>() + +function clearCompletionTimeout(id: string) { + const timeout = completionTimeouts.get(id) + if (timeout) { + clearTimeout(timeout) + completionTimeouts.delete(id) + } +} + export type TimerStatus = 'running' | 'paused' | 'completed' export interface Timer { @@ -40,9 +50,13 @@ export const useTimerStore = create((set) => ({ cleanExpiredTimers: () => { const now = Date.now() set((state) => ({ - timers: state.timers.filter( - (t) => t.status !== 'completed' || now - t.endsAt < COMPLETED_TIMER_CLEANUP_MS - ), + timers: state.timers.filter((t) => { + if (t.status === 'completed' && now - t.endsAt >= COMPLETED_TIMER_CLEANUP_MS) { + clearCompletionTimeout(t.id) + return false + } + return true + }), })) }, @@ -59,6 +73,7 @@ export const useTimerStore = create((set) => ({ }, removeTimer: (id) => { + clearCompletionTimeout(id) set((state) => ({ timers: state.timers.filter((t) => t.id !== id) })) }, @@ -79,14 +94,17 @@ export const useTimerStore = create((set) => ({ const existing = useTimerStore.getState().timers.find((t) => t.id === id) if (!existing || existing.status === 'completed') return + clearCompletionTimeout(id) set((state) => ({ timers: state.timers.map((t) => t.id === id ? { ...t, remainingMs: 0, status: 'completed' } : t ), })) - setTimeout(() => { + const timeout = setTimeout(() => { + completionTimeouts.delete(id) useTimerStore.getState().removeTimer(id) - }, 5000) + }, COMPLETED_TIMER_CLEANUP_MS) + completionTimeouts.set(id, timeout) }, pauseTimer: (id) => { From be9f97eda10129a6b261ddb7ba237c96dd3da390 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 29 Jun 2026 10:09:32 +0000 Subject: [PATCH 2/2] fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit --- src/lib/editor/VariableScope.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/editor/VariableScope.test.ts b/src/lib/editor/VariableScope.test.ts index a0d5dc9..1fbbd44 100644 --- a/src/lib/editor/VariableScope.test.ts +++ b/src/lib/editor/VariableScope.test.ts @@ -43,7 +43,7 @@ describe('VariableScope', () => { it('falls back to raw trimmed string if expression parsing fails', () => { const scopeMgr = new VariableScope() - const doc = '/var greeting = Hello World' + const doc = '/var greeting = Hello World ' scopeMgr.triggerScopeUpdate(doc, null) vi.advanceTimersByTime(300)