From ba7f1e1e3c8aa1214c43fefde04ad4b8583aa580 Mon Sep 17 00:00:00 2001
From: Taylor Mullen <ntaylormullen@google.com>
Date: Mon, 19 May 2025 23:44:24 -0700
Subject: [PATCH] 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
---
 .../components/messages/DiffRenderer.test.tsx | 30 ++++++++-
 .../ui/components/messages/DiffRenderer.tsx   | 62 +++++++++++++++----
 2 files changed, 77 insertions(+), 15 deletions(-)

diff --git a/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx b/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx
index 335ee20a..e3a94f9f 100644
--- a/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx
+++ b/packages/cli/src/ui/components/messages/DiffRenderer.test.tsx
@@ -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');
+  });
 });
diff --git a/packages/cli/src/ui/components/messages/DiffRenderer.tsx b/packages/cli/src/ui/components/messages/DiffRenderer.tsx
index 4baed3e8..bf00537d 100644
--- a/packages/cli/src/ui/components/messages/DiffRenderer.tsx
+++ b/packages/cli/src/ui/components/messages/DiffRenderer.tsx
@@ -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>
   );
 };