refactor(cli): address code review feedback

- Abstracted hardcoded padding, margin, and width calculations in `DenseToolMessage` into explicit constants to improve readability.
- Made `terminalWidth` a required property on `DenseToolMessageProps` to ensure consistent layout calculations, simplifying internal ternary checks.
- Updated `DenseToolMessage` tests to provide the now-required `terminalWidth` prop.
This commit is contained in:
Jarrod Whelan
2026-03-26 15:43:45 -07:00
parent 09a6667c35
commit 6206269149
3 changed files with 30 additions and 31 deletions
+1 -1
View File
@@ -2356,7 +2356,7 @@ conventions and context.
loaded, allowing you to verify the hierarchy and content being used by the
AI.
- See the [Commands documentation](./commands.md#memory) for full details on
the `/memory` command and its sub-commands (`show` and `refresh`).
the `/memory` command and its sub-commands (`show` and `reload`).
By understanding and utilizing these configuration layers and the hierarchical
nature of context files, you can effectively manage the AI's memory and tailor
@@ -1,6 +1,6 @@
/**
* @license
* Copyright 2025 Google LLC
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
@@ -31,6 +31,7 @@ describe('DenseToolMessage', () => {
status: CoreToolCallStatus.Success,
resultDisplay: 'Success result' as ToolResultDisplay,
confirmationDetails: undefined,
terminalWidth: 80,
};
it('renders correctly for a successful string result', async () => {
@@ -1,6 +1,6 @@
/**
* @license
* Copyright 2025 Google LLC
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
@@ -36,8 +36,13 @@ import { colorizeCode } from '../../utils/CodeColorizer.js';
import { useToolActions } from '../../contexts/ToolActionsContext.js';
import { getFileExtension } from '../../utils/fileUtils.js';
const PAYLOAD_MARGIN_LEFT = 6;
const PAYLOAD_BORDER_CHROME_WIDTH = 4; // paddingX=1 (2 cols) + borders (2 cols)
const PAYLOAD_SCROLL_GUTTER = 4;
const PAYLOAD_MAX_WIDTH = 120 + PAYLOAD_SCROLL_GUTTER;
interface DenseToolMessageProps extends IndividualToolCallDisplay {
terminalWidth?: number;
terminalWidth: number;
availableTerminalHeight?: number;
}
@@ -63,10 +68,6 @@ const hasPayload = (res: unknown): res is PayloadResult => {
return typeof value === 'string';
};
/**
* --- RENDER HELPERS ---
*/
const RenderItemsList: React.FC<{
items?: string[];
maxVisible?: number;
@@ -88,15 +89,11 @@ const RenderItemsList: React.FC<{
);
};
/**
* --- SCENARIO LOGIC (Pure Functions) ---
*/
function getFileOpData(
diff: FileDiff,
status: CoreToolCallStatus,
resultDisplay: unknown,
terminalWidth?: number,
terminalWidth: number,
availableTerminalHeight?: number,
isClickable?: boolean,
): ViewParts {
@@ -177,7 +174,7 @@ function getFileOpData(
<DiffRenderer
diffContent={diff.fileDiff}
filename={diff.fileName}
terminalWidth={terminalWidth ? terminalWidth - 6 : 80}
terminalWidth={terminalWidth - PAYLOAD_MARGIN_LEFT}
availableTerminalHeight={availableTerminalHeight}
disableColor={status === CoreToolCallStatus.Cancelled}
/>
@@ -309,10 +306,6 @@ function getGenericSuccessData(
return { description, summary, payload };
}
/**
* --- MAIN COMPONENT ---
*/
export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
const {
callId,
@@ -339,7 +332,7 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
const [isFocused, setIsFocused] = useState(false);
const toggleRef = useRef<DOMElement>(null);
// 1. Unified File Data Extraction (Safely bridge resultDisplay and confirmationDetails)
// Unified File Data Extraction (Safely bridge resultDisplay and confirmationDetails)
const diff = useMemo((): FileDiff | undefined => {
if (isFileDiff(resultDisplay)) return resultDisplay;
if (confirmationDetails?.type === 'edit') {
@@ -375,7 +368,7 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
isActive: isAlternateBuffer && !!diff,
});
// 2. State-to-View Coordination
// State-to-View Coordination
const viewParts = useMemo((): ViewParts => {
if (diff) {
return getFileOpData(
@@ -459,7 +452,7 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
return colorizeCode({
code: addedContent,
language: fileExtension,
maxWidth: terminalWidth ? terminalWidth - 6 : 80,
maxWidth: terminalWidth - PAYLOAD_MARGIN_LEFT,
settings,
disableColor: status === CoreToolCallStatus.Cancelled,
returnLines: true,
@@ -468,7 +461,7 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
return renderDiffLines({
parsedLines,
filename: diff.fileName,
terminalWidth: terminalWidth ? terminalWidth - 6 : 80,
terminalWidth: terminalWidth - PAYLOAD_MARGIN_LEFT,
disableColor: status === CoreToolCallStatus.Cancelled,
});
}
@@ -502,7 +495,6 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
<Box minHeight={1}>{item}</Box>
);
// 3. Final Layout
return (
<Box flexDirection="column">
<Box marginLeft={2} flexDirection="row" flexWrap="wrap">
@@ -529,7 +521,7 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
{showPayload && isAlternateBuffer && diffLines.length > 0 && (
<Box
marginLeft={6}
marginLeft={PAYLOAD_MARGIN_LEFT}
marginTop={1}
marginBottom={1}
paddingX={1}
@@ -541,7 +533,10 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
borderStyle="round"
borderColor={theme.border.default}
borderDimColor={true}
maxWidth={terminalWidth ? Math.min(124, terminalWidth - 6) : 124}
maxWidth={Math.min(
PAYLOAD_MAX_WIDTH,
terminalWidth - PAYLOAD_MARGIN_LEFT,
)}
>
<ScrollableList
data={diffLines}
@@ -549,22 +544,25 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
keyExtractor={keyExtractor}
estimatedItemHeight={() => 1}
hasFocus={isFocused}
width={
// adjustment: 6 margin - 4 padding/border - 4 right-scroll-gutter
terminalWidth ? Math.min(120, terminalWidth - 6 - 4 - 4) : 70
}
width={Math.min(
PAYLOAD_MAX_WIDTH,
terminalWidth -
PAYLOAD_MARGIN_LEFT -
PAYLOAD_BORDER_CHROME_WIDTH -
PAYLOAD_SCROLL_GUTTER,
)}
/>
</Box>
)}
{showPayload && (!isAlternateBuffer || !diff) && viewParts.payload && (
<Box marginLeft={6} marginTop={1} marginBottom={1}>
<Box marginLeft={PAYLOAD_MARGIN_LEFT} marginTop={1} marginBottom={1}>
{viewParts.payload}
</Box>
)}
{showPayload && outputFile && (
<Box marginLeft={6} marginTop={1} marginBottom={1}>
<Box marginLeft={PAYLOAD_MARGIN_LEFT} marginTop={1} marginBottom={1}>
<Text color={theme.text.secondary}>
(Output saved to: {outputFile})
</Text>