Skip to content

Commit c908011

Browse files
ozgesolidkeyclaude
ozgesolidkey
and
claude
committed
Add notes drawer, bookmark colors, scroll fixes, and engineering cleanup
- Notes drawer overlay (Ctrl+Shift+N): slide-up panel with auto-save - Bookmark color palette and improved bookmark modal - Long lines warning with format suggestion on slow scroll detection - Horizontal trackpad scroll support in log viewer - Replace fragile setTimeout animations with transitionend events - Fix transitionend/fallback race condition in overlay hide - Add console.warn to silent catch blocks for debuggability - Store terminal.onData disposable for symmetric cleanup - Guard tab cycling against findIndex returning -1 - Unify highlight creation via shared commitHighlight() - Replace hardcoded width estimation with measured character width - Analyzer improvements for column-aware, drain, and rule-based modes - Add tech debt documentation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent d33e5e0 commit c908011

11 files changed

Lines changed: 1364 additions & 412 deletions

File tree

‎docs/TECH_DEBT.md‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# Technical Debt
2+
3+
Known issues that aren't urgent but should be addressed as the codebase grows.
4+
5+
## Renderer (`src/renderer/renderer.ts`)
6+
7+
### Global Mutable State Without Protection
8+
9+
**Severity**: Medium | **Effort**: Large
10+
11+
The renderer uses ~30+ mutable global variables (`state`, `terminal`, `fitAddon`, `splitDiffState`, `zoomLevel`, etc.) with no encapsulation. Any function can mutate any state, making it hard to trace bugs.
12+
13+
**What to do**: Introduce a state management layer — at minimum a centralized state object with accessor functions that validate mutations. A full reactive/reducer pattern would be ideal but is a significant rewrite.
14+
15+
### Duplicated Modal Pattern
16+
17+
**Severity**: Low | **Effort**: Medium
18+
19+
Multiple modals (highlight modal, bookmark modal, rename modal, etc.) each implement their own promise-based show/hide pattern with `pendingResolve` variables. The pattern is copy-pasted across each modal type.
20+
21+
**What to do**: Create a generic `showModal<T>(element, setup): Promise<T | null>` helper that handles the promise lifecycle, backdrop clicks, Escape key, and resolve/reject. Each modal specializes only its content and result extraction.
22+
23+
### Path Comparison via String Equality
24+
25+
**Severity**: Low | **Effort**: Small
26+
27+
File paths are compared with `===` throughout (e.g., `file.path === state.filePath`). This can fail with path normalization differences, trailing slashes, symlinks, or mixed separators on Windows.
28+
29+
**What to do**: Normalize all paths on entry (e.g., when files are opened) and use a `pathsEqual()` utility for comparisons. Electron's `path` module handles cross-platform normalization.
30+
31+
### `void offsetHeight` Reflow Hacks
32+
33+
**Severity**: Low | **Effort**: Small
34+
35+
Two places use `void overlay.offsetHeight` to force a browser reflow before adding a CSS class, so that CSS transitions trigger from the initial state. This is a well-known browser pattern but is fragile — future CSS changes could make the reflow unnecessary or insufficient.
36+
37+
**What to do**: These are acceptable for now. If they become problematic, replace with a double-`requestAnimationFrame` pattern or use the Web Animations API.
38+
39+
### Feature Coupling: Word Wrap and Markdown Preview
40+
41+
**Severity**: Low | **Effort**: Small
42+
43+
`wordWrapEnabled` boolean is checked in both the word-wrap toggle and markdown preview code paths. These are separate features that share a rendering concern (line wrapping) but aren't the same thing.
44+
45+
**What to do**: If markdown preview evolves to need different wrapping behavior, split into separate state flags.
46+
47+
### Mutation of Filter Rules During Render
48+
49+
**Severity**: Low | **Effort**: Medium
50+
51+
When filter rule types change (e.g., switching from text to level), `updateFilterRule()` mutates state and triggers `renderAdvancedFilterUI()` inline. If another state change happens during that render cycle, the UI can desync.
52+
53+
**What to do**: Batch filter state mutations and render once at the end of the current event loop tick (e.g., via `queueMicrotask`).
54+
55+
### Context Menu `setTimeout(0)` for Click-Outside
56+
57+
**Severity**: Low | **Effort**: Small
58+
59+
Line ~3422 uses `setTimeout(() => document.addEventListener('click', closeMenu), 0)` to defer adding the click-outside handler until after the current click event finishes. This works but is a timing-based workaround.
60+
61+
**What to do**: Use `{ once: true }` on the originating event or `pointer-events` CSS to prevent the immediate re-trigger, removing the need for the setTimeout.

‎src/main/analyzers/columnAwareAnalyzer.ts‎

Lines changed: 135 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import * as fs from 'fs';
2-
import * as readline from 'readline';
32
import {
43
LogAnalyzer,
54
AnalyzerOptions,
@@ -98,111 +97,145 @@ export class ColumnAwareAnalyzer implements LogAnalyzer {
9897
let firstTimestamp: string | null = null;
9998
let lastTimestamp: string | null = null;
10099

101-
const stream = fs.createReadStream(filePath, { encoding: 'utf-8', highWaterMark: 64 * 1024 });
102-
const rl = readline.createInterface({ input: stream, crlfDelay: Infinity });
103-
104-
try {
105-
for await (const line of rl) {
106-
if (signal?.cancelled) break;
107-
108-
lineNumber++;
109-
bytesRead += line.length + 1;
100+
// Use chunked reading instead of readline to prevent OOM on files with
101+
// extremely long lines. readline buffers entire lines in memory.
102+
const MAX_LINE_LENGTH = 500;
103+
const CHUNK_SIZE = 1024 * 1024; // 1MB
104+
const readBuffer = Buffer.alloc(CHUNK_SIZE);
105+
const fd = fs.openSync(filePath, 'r');
106+
let lineBuffer = '';
107+
let lineBufferFull = false;
108+
109+
const processLine = (line: string): void => {
110+
lineNumber++;
111+
bytesRead += line.length + 1;
112+
113+
if (lineNumber === 1 && this.looksLikeHeader(line)) return;
114+
if (!line.trim() || line.startsWith('#')) return;
115+
116+
const fields = this.splitLine(line);
117+
118+
// Extract channel
119+
let channel: string | undefined;
120+
if (channelCol && fields[channelCol.index]) {
121+
channel = fields[channelCol.index].trim();
122+
if (channel && channel !== '--' && channel !== '-') {
123+
channelCounts.set(channel, (channelCounts.get(channel) || 0) + 1);
124+
} else {
125+
channel = undefined;
126+
}
127+
}
110128

111-
if (lineNumber === 1 && this.looksLikeHeader(line)) continue;
112-
if (!line.trim() || line.startsWith('#')) continue;
129+
// Extract source (simplified)
130+
if (sourceCol && fields[sourceCol.index]) {
131+
const source = fields[sourceCol.index].trim();
132+
if (source && source !== '--' && source !== '-') {
133+
const simpleSource = this.simplifySource(source);
134+
sourceCounts.set(simpleSource, (sourceCounts.get(simpleSource) || 0) + 1);
135+
}
136+
}
113137

114-
const fields = this.splitLine(line);
138+
// Extract level
139+
let level: string | undefined;
140+
if (levelCol && fields[levelCol.index]) {
141+
const rawLevel = fields[levelCol.index].trim().toLowerCase();
142+
level = this.normalizeLevel(rawLevel) || undefined;
143+
} else {
144+
level = this.detectLevelFromText(line) || undefined;
145+
}
146+
if (level) levelCounts[level]++;
147+
148+
// Extract and analyze message
149+
let message = '';
150+
if (messageCol && fields[messageCol.index]) {
151+
message = fields[messageCol.index].trim();
152+
} else if (fields.length > 0) {
153+
message = fields[fields.length - 1].trim();
154+
}
115155

116-
// Extract channel
117-
let channel: string | undefined;
118-
if (channelCol && fields[channelCol.index]) {
119-
channel = fields[channelCol.index].trim();
120-
if (channel && channel !== '--' && channel !== '-') {
121-
channelCounts.set(channel, (channelCounts.get(channel) || 0) + 1);
122-
} else {
123-
channel = undefined;
156+
if (message) {
157+
const pattern = this.extractPattern(message);
158+
const existing = messageMap.get(pattern);
159+
160+
if (existing) {
161+
existing.count++;
162+
existing.lastLine = lineNumber;
163+
if (!existing.level && level) existing.level = level;
164+
if (!existing.channel && channel) existing.channel = channel;
165+
} else if (messageMap.size < 50000) {
166+
const info: MessageInfo = {
167+
pattern,
168+
sample: message.length > 200 ? message.substring(0, 200) + '...' : message,
169+
count: 1,
170+
level,
171+
channel,
172+
firstLine: lineNumber,
173+
lastLine: lineNumber
174+
};
175+
messageMap.set(pattern, info);
176+
177+
if (rareMessages.length < 1000) {
178+
rareMessages.push({
179+
text: message.length > 150 ? message.substring(0, 150) + '...' : message,
180+
line: lineNumber,
181+
level,
182+
channel
183+
});
124184
}
125185
}
186+
}
126187

127-
// Extract source (simplified)
128-
if (sourceCol && fields[sourceCol.index]) {
129-
const source = fields[sourceCol.index].trim();
130-
if (source && source !== '--' && source !== '-') {
131-
const simpleSource = this.simplifySource(source);
132-
sourceCounts.set(simpleSource, (sourceCounts.get(simpleSource) || 0) + 1);
133-
}
134-
}
188+
// Timestamp - check first 100 chars only
189+
const tsSample = line.length > 100 ? line.substring(0, 100) : line;
190+
const tsMatch = tsSample.match(/(\d{2}\.\d{2}\.\d{4}\s+\d{2}:\d{2}:\d{2})|(\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2})/);
191+
if (tsMatch) {
192+
if (!firstTimestamp) firstTimestamp = tsMatch[0];
193+
lastTimestamp = tsMatch[0];
194+
}
135195

136-
// Extract level
137-
let level: string | undefined;
138-
if (levelCol && fields[levelCol.index]) {
139-
const rawLevel = fields[levelCol.index].trim().toLowerCase();
140-
level = this.normalizeLevel(rawLevel) || undefined;
141-
} else {
142-
level = this.detectLevelFromText(line) || undefined;
143-
}
144-
if (level) levelCounts[level]++;
145-
146-
// Extract and analyze message
147-
let message = '';
148-
if (messageCol && fields[messageCol.index]) {
149-
message = fields[messageCol.index].trim();
150-
} else if (fields.length > 0) {
151-
// Use last field as message if no message column
152-
message = fields[fields.length - 1].trim();
153-
}
196+
// Progress
197+
const now = Date.now();
198+
if (now - lastProgressUpdate > 200) {
199+
lastProgressUpdate = now;
200+
const percent = Math.round(5 + (bytesRead / fileSize) * 75);
201+
onProgress?.({ phase: 'parsing', percent, message: `Line ${lineNumber.toLocaleString()}...` });
202+
}
203+
};
154204

155-
if (message) {
156-
const pattern = this.extractPattern(message);
157-
const existing = messageMap.get(pattern);
158-
159-
if (existing) {
160-
existing.count++;
161-
existing.lastLine = lineNumber;
162-
if (!existing.level && level) existing.level = level;
163-
if (!existing.channel && channel) existing.channel = channel;
164-
} else if (messageMap.size < 50000) {
165-
const info: MessageInfo = {
166-
pattern,
167-
sample: message.length > 200 ? message.substring(0, 200) + '...' : message,
168-
count: 1,
169-
level,
170-
channel,
171-
firstLine: lineNumber,
172-
lastLine: lineNumber
173-
};
174-
messageMap.set(pattern, info);
175-
176-
// Track potential anomalies (collect first 1000 unique messages)
177-
if (rareMessages.length < 1000) {
178-
rareMessages.push({
179-
text: message.length > 150 ? message.substring(0, 150) + '...' : message,
180-
line: lineNumber,
181-
level,
182-
channel
183-
});
205+
try {
206+
let filePos = 0;
207+
while (filePos < fileSize) {
208+
if (signal?.cancelled) break;
209+
210+
const bytesReadChunk = fs.readSync(fd, readBuffer, 0, CHUNK_SIZE, filePos);
211+
if (bytesReadChunk === 0) break;
212+
filePos += bytesReadChunk;
213+
214+
const chunk = readBuffer.toString('utf-8', 0, bytesReadChunk);
215+
216+
for (let i = 0; i < chunk.length; i++) {
217+
const ch = chunk[i];
218+
if (ch === '\n' || ch === '\r') {
219+
processLine(lineBuffer);
220+
lineBuffer = '';
221+
lineBufferFull = false;
222+
if (ch === '\r' && i + 1 < chunk.length && chunk[i + 1] === '\n') {
223+
i++;
224+
}
225+
} else if (!lineBufferFull) {
226+
lineBuffer += ch;
227+
if (lineBuffer.length >= MAX_LINE_LENGTH) {
228+
lineBufferFull = true;
184229
}
185230
}
186231
}
232+
}
187233

188-
// Timestamp
189-
const tsMatch = line.match(/(\d{2}\.\d{2}\.\d{4}\s+\d{2}:\d{2}:\d{2})|(\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2})/);
190-
if (tsMatch) {
191-
if (!firstTimestamp) firstTimestamp = tsMatch[0];
192-
lastTimestamp = tsMatch[0];
193-
}
194-
195-
// Progress
196-
const now = Date.now();
197-
if (now - lastProgressUpdate > 200) {
198-
lastProgressUpdate = now;
199-
const percent = Math.round(5 + (bytesRead / fileSize) * 75);
200-
onProgress?.({ phase: 'parsing', percent, message: `Line ${lineNumber.toLocaleString()}...` });
201-
}
234+
if (lineBuffer.length > 0) {
235+
processLine(lineBuffer);
202236
}
203237
} finally {
204-
rl.close();
205-
stream.destroy();
238+
fs.closeSync(fd);
206239
}
207240

208241
if (signal?.cancelled) {
@@ -492,7 +525,7 @@ export class ColumnAwareAnalyzer implements LogAnalyzer {
492525
}
493526

494527
private detectLevelFromText(text: string): string | null {
495-
const upper = text.toUpperCase();
528+
const upper = (text.length > 200 ? text.substring(0, 200) : text).toUpperCase();
496529
if (/\b(ERROR|FATAL|CRITICAL|EXCEPTION|PANIC)\b/.test(upper)) return 'error';
497530
if (/\b(WARN|WARNING)\b/.test(upper)) return 'warning';
498531
if (/\b(INFO)\b/.test(upper)) return 'info';
@@ -515,25 +548,24 @@ export class ColumnAwareAnalyzer implements LogAnalyzer {
515548

516549
private async detectColumns(filePath: string): Promise<ColumnInfo[]> {
517550
const columns: ColumnInfo[] = [];
518-
const stream = fs.createReadStream(filePath, { encoding: 'utf-8', highWaterMark: 4096 });
519-
const rl = readline.createInterface({ input: stream, crlfDelay: Infinity });
520551

521-
let lineCount = 0;
552+
// Read first few KB to detect column structure (no readline to avoid OOM on long lines)
553+
const buf = Buffer.alloc(8192);
554+
const fd = fs.openSync(filePath, 'r');
522555
let headerLine: string | null = null;
523-
524556
try {
525-
for await (const line of rl) {
526-
lineCount++;
557+
const bytesRead = fs.readSync(fd, buf, 0, 8192, 0);
558+
const text = buf.toString('utf-8', 0, bytesRead);
559+
const lines = text.split(/\r?\n/).slice(0, 10);
560+
for (const line of lines) {
527561
if (line.startsWith('#')) continue;
528-
if (!headerLine && this.looksLikeHeader(line)) {
562+
if (this.looksLikeHeader(line)) {
529563
headerLine = line;
530564
break;
531565
}
532-
if (lineCount > 5) break;
533566
}
534567
} finally {
535-
rl.close();
536-
stream.destroy();
568+
fs.closeSync(fd);
537569
}
538570

539571
if (!headerLine) return columns;

0 commit comments

Comments
 (0)