mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-20 00:02:11 +00:00
fix: TUI crash with Unicode characters in branch selector
- Use truncateToWidth instead of substring in user-message-selector.ts - Fix truncateToWidth to use Intl.Segmenter for proper grapheme handling - Add tests for Unicode truncation behavior
This commit is contained in:
parent
ed2c182501
commit
240064eec3
4 changed files with 121 additions and 22 deletions
|
|
@ -9,6 +9,7 @@
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
- **Print mode error handling**: `-p` flag now outputs error messages and exits with code 1 when requests fail, instead of silently producing no output.
|
- **Print mode error handling**: `-p` flag now outputs error messages and exits with code 1 when requests fail, instead of silently producing no output.
|
||||||
|
- **Branch selector crash**: Fixed TUI crash when user messages contained Unicode characters (like `✔` or `›`) that caused line width to exceed terminal width. Now uses proper `truncateToWidth` instead of `substring`.
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
import { type Component, Container, Spacer, Text } from "@mariozechner/pi-tui";
|
import { type Component, Container, Spacer, Text, truncateToWidth } from "@mariozechner/pi-tui";
|
||||||
import { theme } from "../theme/theme.js";
|
import { theme } from "../theme/theme.js";
|
||||||
import { DynamicBorder } from "./dynamic-border.js";
|
import { DynamicBorder } from "./dynamic-border.js";
|
||||||
|
|
||||||
|
|
@ -54,8 +54,8 @@ class UserMessageList implements Component {
|
||||||
|
|
||||||
// First line: cursor + message
|
// First line: cursor + message
|
||||||
const cursor = isSelected ? theme.fg("accent", "› ") : " ";
|
const cursor = isSelected ? theme.fg("accent", "› ") : " ";
|
||||||
const maxMsgWidth = width - 2; // Account for cursor
|
const maxMsgWidth = width - 2; // Account for cursor (2 chars)
|
||||||
const truncatedMsg = normalizedMessage.substring(0, maxMsgWidth);
|
const truncatedMsg = truncateToWidth(normalizedMessage, maxMsgWidth);
|
||||||
const messageLine = cursor + (isSelected ? theme.bold(truncatedMsg) : truncatedMsg);
|
const messageLine = cursor + (isSelected ? theme.bold(truncatedMsg) : truncatedMsg);
|
||||||
|
|
||||||
lines.push(messageLine);
|
lines.push(messageLine);
|
||||||
|
|
|
||||||
81
packages/coding-agent/test/truncate-to-width.test.ts
Normal file
81
packages/coding-agent/test/truncate-to-width.test.ts
Normal file
|
|
@ -0,0 +1,81 @@
|
||||||
|
import { truncateToWidth, visibleWidth } from "@mariozechner/pi-tui";
|
||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Tests for truncateToWidth behavior with Unicode characters.
|
||||||
|
*
|
||||||
|
* These tests verify that truncateToWidth properly handles text with
|
||||||
|
* Unicode characters that have different byte vs display widths.
|
||||||
|
*/
|
||||||
|
describe("truncateToWidth", () => {
|
||||||
|
it("should truncate messages with Unicode characters correctly", () => {
|
||||||
|
// This message contains a checkmark (✔) which may have display width > 1 byte
|
||||||
|
const message = '✔ script to run › dev $ concurrently "vite" "node --import tsx ./';
|
||||||
|
const width = 67;
|
||||||
|
const maxMsgWidth = width - 2; // Account for cursor
|
||||||
|
|
||||||
|
const truncated = truncateToWidth(message, maxMsgWidth);
|
||||||
|
const truncatedWidth = visibleWidth(truncated);
|
||||||
|
|
||||||
|
expect(truncatedWidth).toBeLessThanOrEqual(maxMsgWidth);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should handle emoji characters", () => {
|
||||||
|
const message = "🎉 Celebration! 🚀 Launch 📦 Package ready for deployment now";
|
||||||
|
const width = 40;
|
||||||
|
const maxMsgWidth = width - 2;
|
||||||
|
|
||||||
|
const truncated = truncateToWidth(message, maxMsgWidth);
|
||||||
|
const truncatedWidth = visibleWidth(truncated);
|
||||||
|
|
||||||
|
expect(truncatedWidth).toBeLessThanOrEqual(maxMsgWidth);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should handle mixed ASCII and wide characters", () => {
|
||||||
|
const message = "Hello 世界 Test 你好 More text here that is long";
|
||||||
|
const width = 30;
|
||||||
|
const maxMsgWidth = width - 2;
|
||||||
|
|
||||||
|
const truncated = truncateToWidth(message, maxMsgWidth);
|
||||||
|
const truncatedWidth = visibleWidth(truncated);
|
||||||
|
|
||||||
|
expect(truncatedWidth).toBeLessThanOrEqual(maxMsgWidth);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should not truncate messages that fit", () => {
|
||||||
|
const message = "Short message";
|
||||||
|
const width = 50;
|
||||||
|
const maxMsgWidth = width - 2;
|
||||||
|
|
||||||
|
const truncated = truncateToWidth(message, maxMsgWidth);
|
||||||
|
|
||||||
|
expect(truncated).toBe(message);
|
||||||
|
expect(visibleWidth(truncated)).toBeLessThanOrEqual(maxMsgWidth);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should add ellipsis when truncating", () => {
|
||||||
|
const message = "This is a very long message that needs to be truncated";
|
||||||
|
const width = 30;
|
||||||
|
const maxMsgWidth = width - 2;
|
||||||
|
|
||||||
|
const truncated = truncateToWidth(message, maxMsgWidth);
|
||||||
|
|
||||||
|
expect(truncated).toContain("...");
|
||||||
|
expect(visibleWidth(truncated)).toBeLessThanOrEqual(maxMsgWidth);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should handle the exact crash case from issue report", () => {
|
||||||
|
// Terminal width was 67, line had visible width 68
|
||||||
|
// The problematic text contained "✔" and "›" characters
|
||||||
|
const message = '✔ script to run › dev $ concurrently "vite" "node --import tsx ./server.ts"';
|
||||||
|
const terminalWidth = 67;
|
||||||
|
const cursorWidth = 2; // "› " or " "
|
||||||
|
const maxMsgWidth = terminalWidth - cursorWidth;
|
||||||
|
|
||||||
|
const truncated = truncateToWidth(message, maxMsgWidth);
|
||||||
|
const finalWidth = visibleWidth(truncated);
|
||||||
|
|
||||||
|
// The final line (cursor + message) must not exceed terminal width
|
||||||
|
expect(finalWidth + cursorWidth).toBeLessThanOrEqual(terminalWidth);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -309,36 +309,53 @@ export function truncateToWidth(text: string, maxWidth: number, ellipsis: string
|
||||||
return ellipsis.substring(0, maxWidth);
|
return ellipsis.substring(0, maxWidth);
|
||||||
}
|
}
|
||||||
|
|
||||||
let currentWidth = 0;
|
// Separate ANSI codes from visible content using grapheme segmentation
|
||||||
let truncateAt = 0;
|
|
||||||
let i = 0;
|
let i = 0;
|
||||||
|
const segments: Array<{ type: "ansi" | "grapheme"; value: string }> = [];
|
||||||
|
|
||||||
while (i < text.length && currentWidth < targetWidth) {
|
while (i < text.length) {
|
||||||
// Skip ANSI escape sequences (include them in output but don't count width)
|
const ansiResult = extractAnsiCode(text, i);
|
||||||
if (text[i] === "\x1b" && text[i + 1] === "[") {
|
if (ansiResult) {
|
||||||
let j = i + 2;
|
segments.push({ type: "ansi", value: ansiResult.code });
|
||||||
while (j < text.length && !/[a-zA-Z]/.test(text[j]!)) {
|
i += ansiResult.length;
|
||||||
j++;
|
} else {
|
||||||
|
// Find the next ANSI code or end of string
|
||||||
|
let end = i;
|
||||||
|
while (end < text.length) {
|
||||||
|
const nextAnsi = extractAnsiCode(text, end);
|
||||||
|
if (nextAnsi) break;
|
||||||
|
end++;
|
||||||
}
|
}
|
||||||
// Include the final letter of the escape sequence
|
// Segment this non-ANSI portion into graphemes
|
||||||
j++;
|
const textPortion = text.slice(i, end);
|
||||||
truncateAt = j;
|
for (const seg of segmenter.segment(textPortion)) {
|
||||||
i = j;
|
segments.push({ type: "grapheme", value: seg.segment });
|
||||||
|
}
|
||||||
|
i = end;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Build truncated string from segments
|
||||||
|
let result = "";
|
||||||
|
let currentWidth = 0;
|
||||||
|
|
||||||
|
for (const seg of segments) {
|
||||||
|
if (seg.type === "ansi") {
|
||||||
|
result += seg.value;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
const char = text[i]!;
|
const grapheme = seg.value;
|
||||||
const charWidth = visibleWidth(char);
|
const graphemeWidth = visibleWidth(grapheme);
|
||||||
|
|
||||||
if (currentWidth + charWidth > targetWidth) {
|
if (currentWidth + graphemeWidth > targetWidth) {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
currentWidth += charWidth;
|
result += grapheme;
|
||||||
truncateAt = i + 1;
|
currentWidth += graphemeWidth;
|
||||||
i++;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add reset code before ellipsis to prevent styling leaking into it
|
// Add reset code before ellipsis to prevent styling leaking into it
|
||||||
return text.substring(0, truncateAt) + "\x1b[0m" + ellipsis;
|
return result + "\x1b[0m" + ellipsis;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue