mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-01 21:20:44 +00:00
fix(ui): constrain shell output width to prevent box overflow (#2857)
* fix(ui): constrain shell output width to prevent box overflow When shell commands produce wide table output (e.g., gh run list), the text would overflow the bordered box container in the TUI because AnsiOutputText didn't apply any width constraint. This fix: 1. Adds maxWidth prop to AnsiOutputText component 2. Wraps output in MaxSizedBox for proper width/height constraints 3. Adds wrap=truncate to individual text tokens 4. Passes childWidth from ToolMessage (matching other renderers) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ui): address review feedback on AnsiOutput MaxSizedBox wrapping MaxSizedBox requires its direct children to be row <Box> elements; wrapping the rows in an extra <Box flexDirection="column"> broke the layout contract and caused shell output to render as empty content. Remove the wrapper so each line is a direct <Box> child of MaxSizedBox. Update the "handles empty lines and empty tokens" test: with row <Box> elements in place, empty AnsiLines are now correctly preserved as blank output rows (matching the source terminal) instead of being silently collapsed by the former <Text>-per-row rendering. * test(ui): cover multi-token wide-line truncation in AnsiOutputText The existing truncation test used a single 100-char token, which takes the straightforward MaxSizedBox single-segment path. Real-world shell output like `gh run list` is a single logical row composed of many styled-column tokens whose combined width exceeds the box — that path relies on per-token wrap="truncate" plus ink's flex layout for the final crop, not MaxSizedBox itself. Cover that shape so future regressions in either half of the mechanism are caught. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
eae247b50e
commit
9174c11cee
4 changed files with 92 additions and 29 deletions
|
|
@ -29,7 +29,7 @@ describe('<AnsiOutputText />', () => {
|
|||
createAnsiToken({ text: 'world!' }),
|
||||
],
|
||||
];
|
||||
const { lastFrame } = render(<AnsiOutputText data={data} />);
|
||||
const { lastFrame } = render(<AnsiOutputText data={data} maxWidth={80} />);
|
||||
expect(lastFrame()).toBe('Hello, world!');
|
||||
});
|
||||
|
||||
|
|
@ -45,7 +45,7 @@ describe('<AnsiOutputText />', () => {
|
|||
];
|
||||
// Note: ink-testing-library doesn't render styles, so we can only check the text.
|
||||
// We are testing that it renders without crashing.
|
||||
const { lastFrame } = render(<AnsiOutputText data={data} />);
|
||||
const { lastFrame } = render(<AnsiOutputText data={data} maxWidth={80} />);
|
||||
expect(lastFrame()).toBe('BoldItalicUnderlineDimInverse');
|
||||
});
|
||||
|
||||
|
|
@ -58,7 +58,7 @@ describe('<AnsiOutputText />', () => {
|
|||
];
|
||||
// Note: ink-testing-library doesn't render colors, so we can only check the text.
|
||||
// We are testing that it renders without crashing.
|
||||
const { lastFrame } = render(<AnsiOutputText data={data} />);
|
||||
const { lastFrame } = render(<AnsiOutputText data={data} maxWidth={80} />);
|
||||
expect(lastFrame()).toBe('Red FGBlue BG');
|
||||
});
|
||||
|
||||
|
|
@ -69,12 +69,15 @@ describe('<AnsiOutputText />', () => {
|
|||
[createAnsiToken({ text: 'Third line' })],
|
||||
[createAnsiToken({ text: '' })],
|
||||
];
|
||||
const { lastFrame } = render(<AnsiOutputText data={data} />);
|
||||
const { lastFrame } = render(<AnsiOutputText data={data} maxWidth={80} />);
|
||||
const output = lastFrame();
|
||||
expect(output).toBeDefined();
|
||||
const lines = output!.split('\n');
|
||||
expect(lines[0]).toBe('First line');
|
||||
expect(lines[1]).toBe('Third line');
|
||||
// Empty AnsiLines are preserved as blank rows so shell output layout
|
||||
// matches the terminal it came from.
|
||||
expect(lines[1]).toBe('');
|
||||
expect(lines[2]).toBe('Third line');
|
||||
});
|
||||
|
||||
it('respects the availableTerminalHeight prop and slices the lines correctly', () => {
|
||||
|
|
@ -85,7 +88,7 @@ describe('<AnsiOutputText />', () => {
|
|||
[createAnsiToken({ text: 'Line 4' })],
|
||||
];
|
||||
const { lastFrame } = render(
|
||||
<AnsiOutputText data={data} availableTerminalHeight={2} />,
|
||||
<AnsiOutputText data={data} availableTerminalHeight={2} maxWidth={80} />,
|
||||
);
|
||||
const output = lastFrame();
|
||||
expect(output).not.toContain('Line 1');
|
||||
|
|
@ -99,8 +102,48 @@ describe('<AnsiOutputText />', () => {
|
|||
for (let i = 0; i < 1000; i++) {
|
||||
largeData.push([createAnsiToken({ text: `Line ${i}` })]);
|
||||
}
|
||||
const { lastFrame } = render(<AnsiOutputText data={largeData} />);
|
||||
const { lastFrame } = render(
|
||||
<AnsiOutputText data={largeData} maxWidth={80} />,
|
||||
);
|
||||
// We are just checking that it renders something without crashing.
|
||||
expect(lastFrame()).toBeDefined();
|
||||
});
|
||||
|
||||
it('truncates wide lines to fit within maxWidth', () => {
|
||||
const wideText = 'A'.repeat(100);
|
||||
const data: AnsiOutput = [[createAnsiToken({ text: wideText })]];
|
||||
const { lastFrame } = render(<AnsiOutputText data={data} maxWidth={20} />);
|
||||
const output = lastFrame()!;
|
||||
// The line should be truncated to fit within maxWidth
|
||||
expect(output.length).toBeLessThanOrEqual(20);
|
||||
});
|
||||
|
||||
it('truncates multi-token wide lines (styled-column output) to maxWidth', () => {
|
||||
// Mirrors the real-world shape produced by commands like `gh run list`:
|
||||
// a single logical row composed of many styled-column tokens whose
|
||||
// combined width far exceeds the available box width. This exercises
|
||||
// the MaxSizedBox row.segments.length === 0 path, where truncation
|
||||
// depends on per-token wrap="truncate" + ink's flex layout rather
|
||||
// than MaxSizedBox performing the crop itself.
|
||||
const data: AnsiOutput = [
|
||||
[
|
||||
createAnsiToken({ text: 'STATUS ', bold: true }),
|
||||
createAnsiToken({ text: 'TITLE ', bold: true }),
|
||||
createAnsiToken({ text: 'WORKFLOW ', bold: true }),
|
||||
createAnsiToken({ text: 'BRANCH ', bold: true }),
|
||||
createAnsiToken({ text: 'EVENT ', bold: true }),
|
||||
createAnsiToken({ text: 'ID ', bold: true }),
|
||||
createAnsiToken({ text: 'ELAPSED ', bold: true }),
|
||||
createAnsiToken({ text: 'AGE', bold: true }),
|
||||
],
|
||||
];
|
||||
const maxWidth = 30;
|
||||
const { lastFrame } = render(
|
||||
<AnsiOutputText data={data} maxWidth={maxWidth} />,
|
||||
);
|
||||
const output = lastFrame()!;
|
||||
for (const line of output.split('\n')) {
|
||||
expect(line.length).toBeLessThanOrEqual(maxWidth);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -5,46 +5,54 @@
|
|||
*/
|
||||
|
||||
import type React from 'react';
|
||||
import { Text } from 'ink';
|
||||
import { Box, Text } from 'ink';
|
||||
import type {
|
||||
AnsiLine,
|
||||
AnsiOutput,
|
||||
AnsiToken,
|
||||
} from '@qwen-code/qwen-code-core';
|
||||
import { MaxSizedBox } from './shared/MaxSizedBox.js';
|
||||
|
||||
const DEFAULT_HEIGHT = 24;
|
||||
|
||||
interface AnsiOutputProps {
|
||||
data: AnsiOutput;
|
||||
availableTerminalHeight?: number;
|
||||
maxWidth: number;
|
||||
}
|
||||
|
||||
export const AnsiOutputText: React.FC<AnsiOutputProps> = ({
|
||||
data,
|
||||
availableTerminalHeight,
|
||||
maxWidth,
|
||||
}) => {
|
||||
const lastLines = data.slice(
|
||||
-(availableTerminalHeight && availableTerminalHeight > 0
|
||||
? availableTerminalHeight
|
||||
: DEFAULT_HEIGHT),
|
||||
);
|
||||
return lastLines.map((line: AnsiLine, lineIndex: number) => (
|
||||
<Text key={lineIndex}>
|
||||
{line.length > 0
|
||||
? line.map((token: AnsiToken, tokenIndex: number) => (
|
||||
<Text
|
||||
key={tokenIndex}
|
||||
color={token.inverse ? token.bg : token.fg}
|
||||
backgroundColor={token.inverse ? token.fg : token.bg}
|
||||
dimColor={token.dim}
|
||||
bold={token.bold}
|
||||
italic={token.italic}
|
||||
underline={token.underline}
|
||||
>
|
||||
{token.text}
|
||||
</Text>
|
||||
))
|
||||
: null}
|
||||
</Text>
|
||||
));
|
||||
return (
|
||||
<MaxSizedBox maxHeight={availableTerminalHeight} maxWidth={maxWidth}>
|
||||
{lastLines.map((line: AnsiLine, lineIndex: number) => (
|
||||
<Box key={lineIndex}>
|
||||
{line.length > 0
|
||||
? line.map((token: AnsiToken, tokenIndex: number) => (
|
||||
<Text
|
||||
key={tokenIndex}
|
||||
color={token.inverse ? token.bg : token.fg}
|
||||
backgroundColor={token.inverse ? token.fg : token.bg}
|
||||
dimColor={token.dim}
|
||||
bold={token.bold}
|
||||
italic={token.italic}
|
||||
underline={token.underline}
|
||||
wrap="truncate"
|
||||
>
|
||||
{token.text}
|
||||
</Text>
|
||||
))
|
||||
: null}
|
||||
</Box>
|
||||
))}
|
||||
</MaxSizedBox>
|
||||
);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -35,12 +35,22 @@ vi.mock('../TerminalOutput.js', () => ({
|
|||
}));
|
||||
|
||||
vi.mock('../AnsiOutput.js', () => ({
|
||||
AnsiOutputText: function MockAnsiOutputText({ data }: { data: AnsiOutput }) {
|
||||
AnsiOutputText: function MockAnsiOutputText({
|
||||
data,
|
||||
maxWidth,
|
||||
}: {
|
||||
data: AnsiOutput;
|
||||
maxWidth: number;
|
||||
}) {
|
||||
// Simple serialization for snapshot stability
|
||||
const serialized = data
|
||||
.map((line) => line.map((token) => token.text || '').join(''))
|
||||
.join('\n');
|
||||
return <Text>MockAnsiOutput:{serialized}</Text>;
|
||||
return (
|
||||
<Text>
|
||||
MockAnsiOutput:{serialized}:width={maxWidth}
|
||||
</Text>
|
||||
);
|
||||
},
|
||||
}));
|
||||
|
||||
|
|
@ -315,6 +325,7 @@ describe('<ToolMessage />', () => {
|
|||
StreamingState.Idle,
|
||||
);
|
||||
expect(lastFrame()).toContain('MockAnsiOutput:hello');
|
||||
expect(lastFrame()).toContain('width=');
|
||||
});
|
||||
|
||||
it('renders rejected plan content with plan text still visible', () => {
|
||||
|
|
|
|||
|
|
@ -403,6 +403,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
|
|||
<AnsiOutputText
|
||||
data={effectiveDisplayRenderer.data}
|
||||
availableTerminalHeight={availableHeight}
|
||||
maxWidth={innerWidth}
|
||||
/>
|
||||
)}
|
||||
{effectiveDisplayRenderer.type === 'string' && (
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue