mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-09 16:59:43 +02:00
fix: address 7 issues found in xterm-zerolag-input audit
1. Regex global flag safety — strip `g` flag before matching to prevent
lastIndex mutation and missing .index on match results
2. state.visible false before activate — null overlay no longer reports
visible: true
3. removeChar() returns 'pending' | 'flushed' | false instead of boolean
so consumers can distinguish whether to send backspace to PTY
4. removeChar() implements buffer detection cascade (step 3) — detects
existing prompt text when both pending and flushed are empty
5. RenderKey includes text content, not just length — prevents stale
renders when setFlushed() called with same count but different text
6. Remove dead `import type { Terminal }` from test file
7. Remove internal `FontStyle` from public exports
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -40,8 +40,8 @@ terminal.onData((data) => {
|
|||||||
zerolag.clear();
|
zerolag.clear();
|
||||||
ws.send(text + '\r');
|
ws.send(text + '\r');
|
||||||
} else if (data === '\x7f') {
|
} else if (data === '\x7f') {
|
||||||
const removed = zerolag.removeChar();
|
const source = zerolag.removeChar();
|
||||||
if (removed) ws.send(data);
|
if (source === 'flushed') ws.send(data); // only backspace text already in PTY
|
||||||
} else if (data.length === 1 && data.charCodeAt(0) >= 32) {
|
} else if (data.length === 1 && data.charCodeAt(0) >= 32) {
|
||||||
zerolag.addChar(data);
|
zerolag.addChar(data);
|
||||||
// Don't send to server yet — wait for Enter
|
// Don't send to server yet — wait for Enter
|
||||||
@@ -115,7 +115,7 @@ Implements xterm.js `ITerminalAddon`. Load via `terminal.loadAddon(addon)`.
|
|||||||
|--------|-------------|
|
|--------|-------------|
|
||||||
| `addChar(char)` | Add a single printable character to the overlay |
|
| `addChar(char)` | Add a single printable character to the overlay |
|
||||||
| `appendText(text)` | Append multiple characters (e.g., paste) |
|
| `appendText(text)` | Append multiple characters (e.g., paste) |
|
||||||
| `removeChar(): boolean` | Remove last char. Returns `false` if nothing to remove |
|
| `removeChar(): 'pending' \| 'flushed' \| false` | Remove last char. Returns source (`'pending'` = unsent, `'flushed'` = send backspace to PTY) or `false` |
|
||||||
| `clear()` | Clear all state and hide overlay |
|
| `clear()` | Clear all state and hide overlay |
|
||||||
|
|
||||||
#### Flushed Text Tracking
|
#### Flushed Text Tracking
|
||||||
|
|||||||
@@ -7,5 +7,4 @@ export type {
|
|||||||
PromptFinder,
|
PromptFinder,
|
||||||
PromptPosition,
|
PromptPosition,
|
||||||
CellDimensions,
|
CellDimensions,
|
||||||
FontStyle,
|
|
||||||
} from './types.js';
|
} from './types.js';
|
||||||
|
|||||||
@@ -27,11 +27,17 @@ export function findPrompt(
|
|||||||
}
|
}
|
||||||
|
|
||||||
case 'regex': {
|
case 'regex': {
|
||||||
|
// Create a fresh non-global regex to avoid lastIndex mutation
|
||||||
|
// and ensure .match() returns a single result with .index
|
||||||
|
const pattern = finder.pattern;
|
||||||
|
const safePattern = pattern.global
|
||||||
|
? new RegExp(pattern.source, pattern.flags.replace('g', ''))
|
||||||
|
: pattern;
|
||||||
for (let row = terminal.rows - 1; row >= 0; row--) {
|
for (let row = terminal.rows - 1; row >= 0; row--) {
|
||||||
const line = buffer.getLine(viewportTop + row);
|
const line = buffer.getLine(viewportTop + row);
|
||||||
if (!line) continue;
|
if (!line) continue;
|
||||||
const text = line.translateToString(true);
|
const text = line.translateToString(true);
|
||||||
const match = text.match(finder.pattern);
|
const match = text.match(safePattern);
|
||||||
if (match) {
|
if (match) {
|
||||||
const col = match.index ?? 0;
|
const col = match.index ?? 0;
|
||||||
return { row, col };
|
return { row, col };
|
||||||
|
|||||||
@@ -48,7 +48,8 @@ const DEFAULT_CURSOR = '#e0e0e0';
|
|||||||
* zerolag.clear();
|
* zerolag.clear();
|
||||||
* ws.send(text + '\r');
|
* ws.send(text + '\r');
|
||||||
* } else if (data === '\x7f') {
|
* } else if (data === '\x7f') {
|
||||||
* if (zerolag.removeChar()) ws.send(data);
|
* const source = zerolag.removeChar();
|
||||||
|
* if (source === 'flushed') ws.send(data); // only send if text was already in PTY
|
||||||
* } else if (data.length === 1 && data.charCodeAt(0) >= 32) {
|
* } else if (data.length === 1 && data.charCodeAt(0) >= 32) {
|
||||||
* zerolag.addChar(data);
|
* zerolag.addChar(data);
|
||||||
* }
|
* }
|
||||||
@@ -194,14 +195,19 @@ export class ZerolagInputAddon implements XtermAddon {
|
|||||||
* Remove the last character from the overlay.
|
* Remove the last character from the overlay.
|
||||||
*
|
*
|
||||||
* Cascade order:
|
* Cascade order:
|
||||||
* 1. Remove from `pendingText` if non-empty
|
* 1. Remove from `pendingText` if non-empty → returns `'pending'`
|
||||||
* 2. Decrement `flushedOffset` if pending is empty but flushed exists
|
* 2. Decrement `flushedOffset` if pending is empty but flushed exists → returns `'flushed'`
|
||||||
* 3. Try `detectBufferText()` if both are empty
|
* 3. Try `detectBufferText()` if both are empty, then decrement → returns `'flushed'`
|
||||||
*
|
*
|
||||||
* @returns `true` if a character was removed, `false` if nothing to remove.
|
* @returns The source of the removed character, or `false` if nothing to remove.
|
||||||
* When `false`, the consumer should NOT send backspace to the PTY.
|
*
|
||||||
|
* - `'pending'`: A character was removed from unsent text. The consumer
|
||||||
|
* should NOT send backspace to the PTY (the text was never transmitted).
|
||||||
|
* - `'flushed'`: A character was removed from text already sent to the PTY.
|
||||||
|
* The consumer SHOULD send backspace to the PTY.
|
||||||
|
* - `false`: Nothing to remove. The consumer should NOT send backspace.
|
||||||
*/
|
*/
|
||||||
removeChar(): boolean {
|
removeChar(): 'pending' | 'flushed' | false {
|
||||||
if (this._pendingText.length > 0) {
|
if (this._pendingText.length > 0) {
|
||||||
this._pendingText = this._pendingText.slice(0, -1);
|
this._pendingText = this._pendingText.slice(0, -1);
|
||||||
if (this._pendingText.length > 0 || this._flushedOffset > 0) {
|
if (this._pendingText.length > 0 || this._flushedOffset > 0) {
|
||||||
@@ -209,7 +215,7 @@ export class ZerolagInputAddon implements XtermAddon {
|
|||||||
} else {
|
} else {
|
||||||
this._hide();
|
this._hide();
|
||||||
}
|
}
|
||||||
return true;
|
return 'pending';
|
||||||
}
|
}
|
||||||
|
|
||||||
if (this._flushedOffset > 0) {
|
if (this._flushedOffset > 0) {
|
||||||
@@ -220,7 +226,21 @@ export class ZerolagInputAddon implements XtermAddon {
|
|||||||
} else {
|
} else {
|
||||||
this._hide();
|
this._hide();
|
||||||
}
|
}
|
||||||
return true;
|
return 'flushed';
|
||||||
|
}
|
||||||
|
|
||||||
|
// Both empty — try detecting text already on the prompt line
|
||||||
|
// (handles tab completion, arrow-key edits, etc.)
|
||||||
|
this._detectBufferText();
|
||||||
|
if (this._flushedOffset > 0) {
|
||||||
|
this._flushedOffset--;
|
||||||
|
this._flushedText = this._flushedText.slice(0, -1);
|
||||||
|
if (this._flushedOffset > 0) {
|
||||||
|
this._render();
|
||||||
|
} else {
|
||||||
|
this._hide();
|
||||||
|
}
|
||||||
|
return 'flushed';
|
||||||
}
|
}
|
||||||
|
|
||||||
return false;
|
return false;
|
||||||
@@ -369,7 +389,7 @@ export class ZerolagInputAddon implements XtermAddon {
|
|||||||
pendingText: this._pendingText,
|
pendingText: this._pendingText,
|
||||||
flushedLength: this._flushedOffset,
|
flushedLength: this._flushedOffset,
|
||||||
flushedText: this._flushedText,
|
flushedText: this._flushedText,
|
||||||
visible: this._overlay?.style.display !== 'none',
|
visible: this._overlay !== null && this._overlay.style.display !== 'none',
|
||||||
promptPosition: this._lastPromptPos ? { ...this._lastPromptPos } : null,
|
promptPosition: this._lastPromptPos ? { ...this._lastPromptPos } : null,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -500,8 +520,9 @@ export class ZerolagInputAddon implements XtermAddon {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Skip redundant re-renders
|
// Skip redundant re-renders — include text content to detect
|
||||||
const renderKey = `${displayText.length}:${startCol}:${activePrompt.row}:${activePrompt.col}:${totalCols}:${this._flushedOffset}`;
|
// same-length changes (e.g., setFlushed with different text)
|
||||||
|
const renderKey = `${displayText}:${startCol}:${activePrompt.row}:${activePrompt.col}:${totalCols}:${this._flushedOffset}`;
|
||||||
if (renderKey === this._lastRenderKey && this._overlay.style.display !== 'none') return;
|
if (renderKey === this._lastRenderKey && this._overlay.style.display !== 'none') return;
|
||||||
this._lastRenderKey = renderKey;
|
this._lastRenderKey = renderKey;
|
||||||
|
|
||||||
|
|||||||
@@ -88,6 +88,18 @@ describe('findPrompt', () => {
|
|||||||
expect(pos).toBeNull();
|
expect(pos).toBeNull();
|
||||||
cleanup();
|
cleanup();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('handles global flag safely (strips g to avoid lastIndex)', () => {
|
||||||
|
const { terminal, cleanup } = term(['user@host:~$ cmd']);
|
||||||
|
const finder: PromptFinder = { type: 'regex', pattern: /\$/g };
|
||||||
|
const pos = findPrompt(terminal as unknown as XtermTerminal, finder);
|
||||||
|
expect(pos).not.toBeNull();
|
||||||
|
expect(pos!.col).toBe(11);
|
||||||
|
// Call again — should return same result (no lastIndex drift)
|
||||||
|
const pos2 = findPrompt(terminal as unknown as XtermTerminal, finder);
|
||||||
|
expect(pos2).toEqual(pos);
|
||||||
|
cleanup();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('custom strategy', () => {
|
describe('custom strategy', () => {
|
||||||
|
|||||||
@@ -1,7 +1,6 @@
|
|||||||
import { describe, it, expect, afterEach } from 'vitest';
|
import { describe, it, expect, afterEach } from 'vitest';
|
||||||
import { createMockTerminal } from './helpers.js';
|
import { createMockTerminal } from './helpers.js';
|
||||||
import { ZerolagInputAddon } from '../src/zerolag-input-addon.js';
|
import { ZerolagInputAddon } from '../src/zerolag-input-addon.js';
|
||||||
import type { Terminal } from '../src/types.js';
|
|
||||||
|
|
||||||
function setup(lines: string[] = ['$ '], promptChar = '$') {
|
function setup(lines: string[] = ['$ '], promptChar = '$') {
|
||||||
const mock = createMockTerminal({ buffer: { lines } });
|
const mock = createMockTerminal({ buffer: { lines } });
|
||||||
@@ -83,12 +82,12 @@ describe('ZerolagInputAddon', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe('removeChar', () => {
|
describe('removeChar', () => {
|
||||||
it('removes last character from pendingText', () => {
|
it('returns "pending" when removing from pendingText', () => {
|
||||||
const { addon } = tracked();
|
const { addon } = tracked();
|
||||||
addon.addChar('a');
|
addon.addChar('a');
|
||||||
addon.addChar('b');
|
addon.addChar('b');
|
||||||
const removed = addon.removeChar();
|
const source = addon.removeChar();
|
||||||
expect(removed).toBe(true);
|
expect(source).toBe('pending');
|
||||||
expect(addon.pendingText).toBe('a');
|
expect(addon.pendingText).toBe('a');
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -97,10 +96,11 @@ describe('ZerolagInputAddon', () => {
|
|||||||
expect(addon.removeChar()).toBe(false);
|
expect(addon.removeChar()).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('decrements flushed when pending is empty', () => {
|
it('returns "flushed" when removing from flushed text', () => {
|
||||||
const { addon } = tracked();
|
const { addon } = tracked();
|
||||||
addon.setFlushed(3, 'abc');
|
addon.setFlushed(3, 'abc');
|
||||||
expect(addon.removeChar()).toBe(true);
|
const source = addon.removeChar();
|
||||||
|
expect(source).toBe('flushed');
|
||||||
expect(addon.getFlushed().count).toBe(2);
|
expect(addon.getFlushed().count).toBe(2);
|
||||||
expect(addon.getFlushed().text).toBe('ab');
|
expect(addon.getFlushed().text).toBe('ab');
|
||||||
});
|
});
|
||||||
@@ -109,7 +109,8 @@ describe('ZerolagInputAddon', () => {
|
|||||||
const { addon } = tracked();
|
const { addon } = tracked();
|
||||||
addon.setFlushed(2, 'ab');
|
addon.setFlushed(2, 'ab');
|
||||||
addon.addChar('c');
|
addon.addChar('c');
|
||||||
expect(addon.removeChar()).toBe(true);
|
const source = addon.removeChar();
|
||||||
|
expect(source).toBe('pending');
|
||||||
expect(addon.pendingText).toBe('');
|
expect(addon.pendingText).toBe('');
|
||||||
expect(addon.getFlushed().count).toBe(2); // flushed unchanged
|
expect(addon.getFlushed().count).toBe(2); // flushed unchanged
|
||||||
});
|
});
|
||||||
@@ -120,6 +121,21 @@ describe('ZerolagInputAddon', () => {
|
|||||||
addon.removeChar();
|
addon.removeChar();
|
||||||
expect(addon.hasPending).toBe(false);
|
expect(addon.hasPending).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('detects buffer text and removes from it when both empty', () => {
|
||||||
|
const { addon } = tracked(['$ hello']);
|
||||||
|
// Both pending and flushed are empty, but buffer has text
|
||||||
|
const source = addon.removeChar();
|
||||||
|
expect(source).toBe('flushed');
|
||||||
|
// "hello" (5 chars) detected, then one removed = 4
|
||||||
|
expect(addon.getFlushed().count).toBe(4);
|
||||||
|
expect(addon.getFlushed().text).toBe('hell');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns false on empty prompt with no buffer text', () => {
|
||||||
|
const { addon } = tracked(['$ ']);
|
||||||
|
expect(addon.removeChar()).toBe(false);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('clear', () => {
|
describe('clear', () => {
|
||||||
@@ -276,6 +292,22 @@ describe('ZerolagInputAddon', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('state.visible', () => {
|
||||||
|
it('is false before activate', () => {
|
||||||
|
const addon = new ZerolagInputAddon();
|
||||||
|
expect(addon.state.visible).toBe(false);
|
||||||
|
// No cleanup needed — never activated
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is false after dispose', () => {
|
||||||
|
const { addon, mock } = tracked();
|
||||||
|
addon.addChar('x');
|
||||||
|
addon.dispose();
|
||||||
|
expect(addon.state.visible).toBe(false);
|
||||||
|
mock.cleanup();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('rerender / refreshFont', () => {
|
describe('rerender / refreshFont', () => {
|
||||||
it('rerender does not crash when no text', () => {
|
it('rerender does not crash when no text', () => {
|
||||||
const { addon } = tracked();
|
const { addon } = tracked();
|
||||||
|
|||||||
Reference in New Issue
Block a user