Skip to content

Commit 0317342

Browse files
committed
refactor: deduplicate shortcuts and manage timer completion timeouts
1 parent 5feed52 commit 0317342

5 files changed

Lines changed: 113 additions & 31 deletions

File tree

AUDIT_LOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,18 @@
22

33
This log tracks all significant changes, updates, and versions in the PaperCache project.
44

5+
## 2026-06-29 (Code Quality Refactor & Test Suite)
6+
**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`
7+
8+
**Details/Why:**
9+
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`.
10+
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.
11+
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.
12+
13+
**Files changed:** `src-tauri/src/commands/shortcuts.rs`, `src/store/useTimerStore.ts`, `src/lib/editor/VariableScope.test.ts`, `AUDIT_LOG.md`, `CHANGELOG.md`.
14+
15+
---
16+
517
## 2026-06-29 (Code Quality Cleanup)
618
**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
719

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ All notable, user-facing changes to PaperCache will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [Unreleased]
9+
10+
### Changed
11+
- **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`).
12+
813
## [v0.5.6] - 2026-06-28
914

1015
### Added

src-tauri/src/commands/shortcuts.rs

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,22 @@ impl Default for GlobalShortcutState {
1515
}
1616
}
1717

18+
fn handle_shortcut_trigger(app: &AppHandle, action: &str) {
19+
if action == "new-note" {
20+
if let Some(window) = app.get_webview_window("main") {
21+
if !window.is_visible().unwrap_or(false) {
22+
let _ = window.show();
23+
let _ = window.set_focus();
24+
#[cfg(target_os = "macos")]
25+
crate::macos::force_focus();
26+
}
27+
}
28+
} else {
29+
crate::commands::system::toggle_window(app);
30+
}
31+
let _ = app.emit(&format!("trigger-{}", action), ());
32+
}
33+
1834
#[tauri::command]
1935
pub fn update_global_shortcut(
2036
app: AppHandle,
@@ -39,19 +55,7 @@ pub fn update_global_shortcut(
3955
app.global_shortcut()
4056
.on_shortcut(shortcut, move |app, _shortcut, event| {
4157
if event.state() == ShortcutState::Pressed {
42-
if action_clone == "new-note" {
43-
if let Some(window) = app.get_webview_window("main") {
44-
if !window.is_visible().unwrap_or(false) {
45-
let _ = window.show();
46-
let _ = window.set_focus();
47-
#[cfg(target_os = "macos")]
48-
crate::macos::force_focus();
49-
}
50-
}
51-
} else {
52-
crate::commands::system::toggle_window(app);
53-
}
54-
let _ = app.emit(&format!("trigger-{}", action_clone), ());
58+
handle_shortcut_trigger(app, &action_clone);
5559
}
5660
})
5761
.map_err(|e| format!("Failed to register shortcut: {}", e))?;
@@ -84,19 +88,7 @@ pub fn resume_shortcuts(app: AppHandle) -> Result<(), String> {
8488
.global_shortcut()
8589
.on_shortcut(shortcut, move |app, _, event| {
8690
if event.state() == ShortcutState::Pressed {
87-
if action_clone == "new-note" {
88-
if let Some(window) = app.get_webview_window("main") {
89-
if !window.is_visible().unwrap_or(false) {
90-
let _ = window.show();
91-
let _ = window.set_focus();
92-
#[cfg(target_os = "macos")]
93-
crate::macos::force_focus();
94-
}
95-
}
96-
} else {
97-
crate::commands::system::toggle_window(app);
98-
}
99-
let _ = app.emit(&format!("trigger-{}", action_clone), ());
91+
handle_shortcut_trigger(app, &action_clone);
10092
}
10193
});
10294
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'
2+
import { VariableScope, getScope } from './VariableScope'
3+
import { useVariableStore } from '../../store/useVariableStore'
4+
5+
describe('VariableScope', () => {
6+
beforeEach(() => {
7+
vi.useFakeTimers()
8+
useVariableStore.getState().setGlobals({})
9+
useVariableStore.getState().setNoteScope({})
10+
})
11+
12+
afterEach(() => {
13+
vi.useRealTimers()
14+
})
15+
16+
it('merges global and note scopes in getScope', () => {
17+
useVariableStore.getState().setGlobals({ globalA: 10, shared: 'global' })
18+
useVariableStore.getState().setNoteScope({ noteB: 20, shared: 'note' })
19+
20+
const scope = getScope()
21+
expect(scope).toEqual({
22+
globalA: 10,
23+
noteB: 20,
24+
shared: 'note',
25+
})
26+
})
27+
28+
it('parses mathematical expressions and updates note scope after debounce', () => {
29+
const scopeMgr = new VariableScope()
30+
const doc = '/var x = 10 + 5\n/var y = x * 2'
31+
32+
scopeMgr.triggerScopeUpdate(doc, null)
33+
34+
expect(useVariableStore.getState().getNoteScope()).toEqual({})
35+
36+
vi.advanceTimersByTime(300)
37+
38+
expect(useVariableStore.getState().getNoteScope()).toEqual({
39+
x: 15,
40+
y: 30,
41+
})
42+
})
43+
44+
it('falls back to raw trimmed string if expression parsing fails', () => {
45+
const scopeMgr = new VariableScope()
46+
const doc = '/var greeting = Hello World'
47+
48+
scopeMgr.triggerScopeUpdate(doc, null)
49+
vi.advanceTimersByTime(300)
50+
51+
expect(useVariableStore.getState().getNoteScope()).toEqual({
52+
greeting: 'Hello World',
53+
})
54+
})
55+
})

src/store/useTimerStore.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@ import { create } from 'zustand'
1010

1111
const COMPLETED_TIMER_CLEANUP_MS = 10000
1212

13+
const completionTimeouts = new Map<string, ReturnType<typeof setTimeout>>()
14+
15+
function clearCompletionTimeout(id: string) {
16+
const timeout = completionTimeouts.get(id)
17+
if (timeout) {
18+
clearTimeout(timeout)
19+
completionTimeouts.delete(id)
20+
}
21+
}
22+
1323
export type TimerStatus = 'running' | 'paused' | 'completed'
1424

1525
export interface Timer {
@@ -40,9 +50,13 @@ export const useTimerStore = create<TimerState>((set) => ({
4050
cleanExpiredTimers: () => {
4151
const now = Date.now()
4252
set((state) => ({
43-
timers: state.timers.filter(
44-
(t) => t.status !== 'completed' || now - t.endsAt < COMPLETED_TIMER_CLEANUP_MS
45-
),
53+
timers: state.timers.filter((t) => {
54+
if (t.status === 'completed' && now - t.endsAt >= COMPLETED_TIMER_CLEANUP_MS) {
55+
clearCompletionTimeout(t.id)
56+
return false
57+
}
58+
return true
59+
}),
4660
}))
4761
},
4862

@@ -59,6 +73,7 @@ export const useTimerStore = create<TimerState>((set) => ({
5973
},
6074

6175
removeTimer: (id) => {
76+
clearCompletionTimeout(id)
6277
set((state) => ({ timers: state.timers.filter((t) => t.id !== id) }))
6378
},
6479

@@ -79,14 +94,17 @@ export const useTimerStore = create<TimerState>((set) => ({
7994
const existing = useTimerStore.getState().timers.find((t) => t.id === id)
8095
if (!existing || existing.status === 'completed') return
8196

97+
clearCompletionTimeout(id)
8298
set((state) => ({
8399
timers: state.timers.map((t) =>
84100
t.id === id ? { ...t, remainingMs: 0, status: 'completed' } : t
85101
),
86102
}))
87-
setTimeout(() => {
103+
const timeout = setTimeout(() => {
104+
completionTimeouts.delete(id)
88105
useTimerStore.getState().removeTimer(id)
89-
}, 5000)
106+
}, COMPLETED_TIMER_CLEANUP_MS)
107+
completionTimeouts.set(id, timeout)
90108
},
91109

92110
pauseTimer: (id) => {

0 commit comments

Comments
 (0)