feat: Improve diff rendering with gap indicators

- Adds a visual indicator for skipped lines in the diff view.
- Updates tests to verify gap indicator rendering.
- Adjusts line number padding for better alignment.

Fixes https://b.corp.google.com/issues/414453107
This commit is contained in:
Taylor Mullen 2025-05-19 23:44:24 -07:00 committed by N. Taylor Mullen
parent 872f308536
commit ba7f1e1e3c
2 changed files with 77 additions and 15 deletions

View File

@ -92,8 +92,8 @@ index 0000001..0000002 100644
);
const output = lastFrame();
const lines = output!.split('\n');
expect(lines[0]).toBe('1 - old line');
expect(lines[1]).toBe('1 + new line');
expect(lines[0]).toBe('1 - old line');
expect(lines[1]).toBe('1 + new line');
});
it('should handle diff with only header and no changes', () => {
@ -114,4 +114,30 @@ index 1234567..1234567 100644
expect(lastFrame()).toContain('No diff content');
expect(mockColorizeCode).not.toHaveBeenCalled();
});
it('should render a gap indicator for skipped lines', () => {
const diffWithGap = `
diff --git a/file.txt b/file.txt
index 123..456 100644
--- a/file.txt
+++ b/file.txt
@@ -1,2 +1,2 @@
context line 1
-deleted line
+added line
@@ -10,2 +10,2 @@
context line 10
context line 11
`;
const { lastFrame } = render(
<DiffRenderer diffContent={diffWithGap} filename="file.txt" />,
);
const output = lastFrame();
expect(output).toContain('═'); // Check for the border character used in the gap
// Verify that lines before and after the gap are rendered
expect(output).toContain('context line 1');
expect(output).toContain('added line');
expect(output).toContain('context line 10');
});
});

View File

@ -187,11 +187,42 @@ const renderDiffContent = (
? `diff-box-${filename}`
: `diff-box-${crypto.createHash('sha1').update(JSON.stringify(parsedLines)).digest('hex')}`;
let lastLineNumber: number | null = null;
const MAX_CONTEXT_LINES_WITHOUT_GAP = 1;
return (
<Box flexDirection="column" key={key}>
{/* Iterate over the lines that should be displayed (already normalized) */}
{displayableLines.map((line, index) => {
const key = `diff-line-${index}`;
{displayableLines.reduce<React.ReactNode[]>((acc, line, index) => {
// Determine the relevant line number for gap calculation based on type
let relevantLineNumberForGapCalc: number | null = null;
if (line.type === 'add' || line.type === 'context') {
relevantLineNumberForGapCalc = line.newLine ?? null;
} else if (line.type === 'del') {
// For deletions, the gap is typically in relation to the original file's line numbering
relevantLineNumberForGapCalc = line.oldLine ?? null;
}
if (
lastLineNumber !== null &&
relevantLineNumberForGapCalc !== null &&
relevantLineNumberForGapCalc >
lastLineNumber + MAX_CONTEXT_LINES_WITHOUT_GAP + 1
) {
acc.push(
<Box
key={`gap-${index}`}
width="100%"
borderTop={true}
borderBottom={false}
borderRight={false}
borderLeft={false}
borderStyle="double"
borderColor={Colors.SubtleComment}
></Box>,
);
}
const lineKey = `diff-line-${index}`;
let gutterNumStr = '';
let color: string | undefined = undefined;
let prefixSymbol = ' ';
@ -202,39 +233,44 @@ const renderDiffContent = (
gutterNumStr = (line.newLine ?? '').toString();
color = 'green';
prefixSymbol = '+';
lastLineNumber = line.newLine ?? null;
break;
case 'del':
gutterNumStr = (line.oldLine ?? '').toString();
color = 'red';
prefixSymbol = '-';
// For deletions, update lastLineNumber based on oldLine if it's advancing.
// This helps manage gaps correctly if there are multiple consecutive deletions
// or if a deletion is followed by a context line far away in the original file.
if (line.oldLine !== undefined) {
lastLineNumber = line.oldLine;
}
break;
case 'context':
// Show new line number for context lines in gutter
gutterNumStr = (line.newLine ?? '').toString();
dim = true;
prefixSymbol = ' ';
lastLineNumber = line.newLine ?? null;
break;
default:
throw new Error(`Unknown line type: ${line.type}`);
return acc;
}
// Render the line content *after* stripping the calculated *minimum* baseIndentation.
// The line.content here is already the tab-normalized version.
const displayContent = line.content.substring(baseIndentation);
return (
// Using your original rendering structure
<Box key={key} flexDirection="row">
<Text color={Colors.Foreground}>{gutterNumStr} </Text>
acc.push(
<Box key={lineKey} flexDirection="row">
<Text color={Colors.Foreground}>{gutterNumStr.padEnd(4)} </Text>
<Text color={color} dimColor={dim}>
{prefixSymbol}{' '}
</Text>
<Text color={color} dimColor={dim} wrap="wrap">
{displayContent}
</Text>
</Box>
</Box>,
);
})}
return acc;
}, [])}
</Box>
);
};