Merge pull request #1561 from BlockHand/show-line-numbers

feat: make DiffRenderer respect ui.showLineNumbers setting

packages/cli/src/ui/components/ThemeDialog.tsx

@@ -260,6 +260,7 @@ def fibonacci(n):
                     availableTerminalHeight={diffHeight}
                     contentWidth={colorizeCodeWidth}
                     theme={previewTheme}
+                    settings={settings}
                   />
                 </Box>
               );

packages/cli/src/ui/components/messages/DiffRenderer.test.tsx

@@ -9,6 +9,15 @@ import { render } from 'ink-testing-library';
 import { DiffRenderer } from './DiffRenderer.js';
 import * as CodeColorizer from '../../utils/CodeColorizer.js';
 import { vi } from 'vitest';
+import type { LoadedSettings } from '../../../config/settings.js';
+
+const mockSettings: LoadedSettings = {
+  merged: {
+    ui: {
+      showLineNumbers: true,
+    },
+  },
+} as LoadedSettings;
 
 describe('<OverflowProvider><DiffRenderer /></OverflowProvider>', () => {
   const mockColorizeCode = vi.spyOn(CodeColorizer, 'colorizeCode');
@@ -17,8 +26,8 @@ describe('<OverflowProvider><DiffRenderer /></OverflowProvider>', () => {
     mockColorizeCode.mockClear();
   });
 
-  const sanitizeOutput = (output: string | undefined, terminalWidth: number) =>
-    output?.replace(/GAP_INDICATOR/g, '═'.repeat(terminalWidth));
+  const sanitizeOutput = (output: string | undefined, contentWidth: number) =>
+    output?.replace(/GAP_INDICATOR/g, '═'.repeat(contentWidth));
 
   it('should call colorizeCode with correct language for new file with known extension', () => {
     const newFileDiffContent = `
@@ -36,6 +45,7 @@ index 0000000..e69de29
           diffContent={newFileDiffContent}
           filename="test.py"
           contentWidth={80}
+          settings={mockSettings}
         />
       </OverflowProvider>,
     );
@@ -45,6 +55,7 @@ index 0000000..e69de29
       undefined,
       80,
       undefined,
+      mockSettings,
     );
   });
 
@@ -64,6 +75,7 @@ index 0000000..e69de29
           diffContent={newFileDiffContent}
           filename="test.unknown"
           contentWidth={80}
+          settings={mockSettings}
         />
       </OverflowProvider>,
     );
@@ -73,6 +85,7 @@ index 0000000..e69de29
       undefined,
       80,
       undefined,
+      mockSettings,
     );
   });
 
@@ -88,7 +101,11 @@ index 0000000..e69de29
 `;
     render(
       <OverflowProvider>
-        <DiffRenderer diffContent={newFileDiffContent} contentWidth={80} />
+        <DiffRenderer
+          diffContent={newFileDiffContent}
+          contentWidth={80}
+          settings={mockSettings}
+        />
       </OverflowProvider>,
     );
     expect(mockColorizeCode).toHaveBeenCalledWith(
@@ -97,6 +114,7 @@ index 0000000..e69de29
       undefined,
       80,
       undefined,
+      mockSettings,
     );
   });
 
@@ -116,6 +134,7 @@ index 0000001..0000002 100644
           diffContent={existingFileDiffContent}
           filename="test.txt"
           contentWidth={80}
+          settings={mockSettings}
         />
       </OverflowProvider>,
     );
@@ -146,6 +165,7 @@ index 1234567..1234567 100644
           diffContent={noChangeDiff}
           filename="file.txt"
           contentWidth={80}
+          settings={mockSettings}
         />
       </OverflowProvider>,
     );
@@ -156,7 +176,11 @@ index 1234567..1234567 100644
   it('should handle empty diff content', () => {
     const { lastFrame } = render(
       <OverflowProvider>
-        <DiffRenderer diffContent="" contentWidth={80} />
+        <DiffRenderer
+          diffContent=""
+          contentWidth={80}
+          settings={mockSettings}
+        />
       </OverflowProvider>,
     );
     expect(lastFrame()).toContain('No diff content');
@@ -183,6 +207,7 @@ index 123..456 100644
           diffContent={diffWithGap}
           filename="file.txt"
           contentWidth={80}
+          settings={mockSettings}
         />
       </OverflowProvider>,
     );
@@ -220,6 +245,7 @@ index abc..def 100644
           diffContent={diffWithSmallGap}
           filename="file.txt"
           contentWidth={80}
+          settings={mockSettings}
         />
       </OverflowProvider>,
     );
@@ -251,7 +277,7 @@ index 123..789 100644
 
     it.each([
       {
-        terminalWidth: 80,
+        contentWidth: 80,
         height: undefined,
         expected: ` 1   console.log('first hunk');
  2 - const oldVar = 1;
@@ -264,7 +290,7 @@ index 123..789 100644
 22   console.log('end of second hunk');`,
       },
       {
-        terminalWidth: 80,
+        contentWidth: 80,
         height: 6,
         expected: `... first 4 lines hidden ...
 ════════════════════════════════════════════════════════════════════════════════
@@ -274,7 +300,7 @@ index 123..789 100644
 22   console.log('end of second hunk');`,
       },
       {
-        terminalWidth: 30,
+        contentWidth: 30,
         height: 6,
         expected: `... first 10 lines hidden ...
    ;
@@ -284,20 +310,21 @@ index 123..789 100644
      second hunk');`,
       },
     ])(
-      'with terminalWidth $terminalWidth and height $height',
-      ({ terminalWidth, height, expected }) => {
+      'with contentWidth $contentWidth and height $height',
+      ({ contentWidth, height, expected }) => {
         const { lastFrame } = render(
           <OverflowProvider>
             <DiffRenderer
               diffContent={diffWithMultipleHunks}
               filename="multi.js"
-              contentWidth={terminalWidth}
+              contentWidth={contentWidth}
               availableTerminalHeight={height}
+              settings={mockSettings}
             />
           </OverflowProvider>,
         );
         const output = lastFrame();
-        expect(sanitizeOutput(output, terminalWidth)).toEqual(expected);
+        expect(sanitizeOutput(output, contentWidth)).toEqual(expected);
       },
     );
   });
@@ -324,6 +351,7 @@ fileDiff Index: file.txt
           diffContent={newFileDiff}
           filename="TEST"
           contentWidth={80}
+          settings={mockSettings}
         />
       </OverflowProvider>,
     );
@@ -354,6 +382,7 @@ fileDiff Index: Dockerfile
           diffContent={newFileDiff}
           filename="Dockerfile"
           contentWidth={80}
+          settings={mockSettings}
         />
       </OverflowProvider>,
     );
@@ -362,4 +391,86 @@ fileDiff Index: Dockerfile
 2 RUN npm install
 3 RUN npm run build`);
   });
+
+  describe('showLineNumbers setting', () => {
+    const diffContent = `
+diff --git a/test.txt b/test.txt
+index 0000001..0000002 100644
+--- a/test.txt
++++ b/test.txt
+@@ -1,2 +1,2 @@
+-old line 1
++new line 1
+ context line 2
+`;
+
+    it('should show line numbers by default when settings is undefined', () => {
+      const { lastFrame } = render(
+        <OverflowProvider>
+          <DiffRenderer
+            diffContent={diffContent}
+            filename="test.txt"
+            contentWidth={80}
+          />
+        </OverflowProvider>,
+      );
+      const output = lastFrame();
+      expect(output).toContain('1 -');
+      expect(output).toContain('1 +');
+      expect(output).toContain('2  ');
+    });
+
+    it('should show line numbers when showLineNumbers is true', () => {
+      const mockSettings = {
+        merged: {
+          ui: {
+            showLineNumbers: true,
+          },
+        },
+      } as unknown as LoadedSettings;
+
+      const { lastFrame } = render(
+        <OverflowProvider>
+          <DiffRenderer
+            diffContent={diffContent}
+            filename="test.txt"
+            contentWidth={80}
+            settings={mockSettings}
+          />
+        </OverflowProvider>,
+      );
+      const output = lastFrame();
+      expect(output).toContain('1 -');
+      expect(output).toContain('1 +');
+      expect(output).toContain('2  ');
+    });
+
+    it('should hide line numbers when showLineNumbers is false', () => {
+      const mockSettings = {
+        merged: {
+          ui: {
+            showLineNumbers: false,
+          },
+        },
+      } as unknown as LoadedSettings;
+
+      const { lastFrame } = render(
+        <OverflowProvider>
+          <DiffRenderer
+            diffContent={diffContent}
+            filename="test.txt"
+            contentWidth={80}
+            settings={mockSettings}
+          />
+        </OverflowProvider>,
+      );
+      const output = lastFrame();
+      // Line numbers should not be present
+      expect(output).not.toMatch(/^\s*\d+\s*[-+]/m);
+      // But the content should still be there
+      expect(output).toContain('old line 1');
+      expect(output).toContain('new line 1');
+      expect(output).toContain('context line 2');
+    });
+  });
 });

packages/cli/src/ui/components/messages/DiffRenderer.tsx

@@ -11,6 +11,7 @@ import { colorizeCode, colorizeLine } from '../../utils/CodeColorizer.js';
 import { MaxSizedBox } from '../shared/MaxSizedBox.js';
 import { theme as semanticTheme } from '../../semantic-colors.js';
 import type { Theme } from '../../themes/theme.js';
+import type { LoadedSettings } from '../../../config/settings.js';
 
 interface DiffLine {
   type: 'add' | 'del' | 'context' | 'hunk' | 'other';
@@ -86,6 +87,7 @@ interface DiffRendererProps {
   availableTerminalHeight?: number;
   contentWidth: number;
   theme?: Theme;
+  settings?: LoadedSettings;
 }
 
 const DEFAULT_TAB_WIDTH = 4; // Spaces per tab for normalization
@@ -97,6 +99,7 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
   availableTerminalHeight,
   contentWidth,
   theme,
+  settings,
 }) => {
   const screenReaderEnabled = useIsScreenReaderEnabled();
   if (!diffContent || typeof diffContent !== 'string') {
@@ -157,6 +160,7 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
       availableTerminalHeight,
       contentWidth,
       theme,
+      settings,
     );
   } else {
     renderedOutput = renderDiffContent(
@@ -165,6 +169,7 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
       tabWidth,
       availableTerminalHeight,
       contentWidth,
+      settings,
     );
   }
 
@@ -177,6 +182,7 @@ const renderDiffContent = (
   tabWidth = DEFAULT_TAB_WIDTH,
   availableTerminalHeight: number | undefined,
   contentWidth: number,
+  settings?: LoadedSettings,
 ) => {
   // 1. Normalize whitespace (replace tabs with spaces) *before* further processing
   const normalizedLines = parsedLines.map((line) => ({
@@ -201,6 +207,8 @@ const renderDiffContent = (
     );
   }
 
+  const showLineNumbers = settings?.merged.ui?.showLineNumbers ?? true;
+
   const maxLineNumber = Math.max(
     0,
     ...displayableLines.map((l) => l.oldLine ?? 0),
@@ -299,18 +307,20 @@ const renderDiffContent = (
 
         acc.push(
           <Box key={lineKey} flexDirection="row">
-            <Text
-              color={semanticTheme.text.secondary}
-              backgroundColor={
-                line.type === 'add'
-                  ? semanticTheme.background.diff.added
-                  : line.type === 'del'
-                    ? semanticTheme.background.diff.removed
-                    : undefined
-              }
-            >
-              {gutterNumStr.padStart(gutterWidth)}{' '}
-            </Text>
+            {showLineNumbers && (
+              <Text
+                color={semanticTheme.text.secondary}
+                backgroundColor={
+                  line.type === 'add'
+                    ? semanticTheme.background.diff.added
+                    : line.type === 'del'
+                      ? semanticTheme.background.diff.removed
+                      : undefined
+                }
+              >
+                {gutterNumStr.padStart(gutterWidth)}{' '}
+              </Text>
+            )}
             {line.type === 'context' ? (
               <>
                 <Text>{prefixSymbol} </Text>

packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx

@@ -226,6 +226,7 @@ export const ToolConfirmationMessage: React.FC<
         filename={confirmationDetails.fileName}
         availableTerminalHeight={availableBodyContentHeight()}
         contentWidth={contentWidth}
+        settings={settings}
       />
     );
   } else if (confirmationDetails.type === 'exec') {

packages/cli/src/ui/components/messages/ToolMessage.test.tsx

@@ -11,11 +11,13 @@ import { ToolMessage } from './ToolMessage.js';
 import { StreamingState, ToolCallStatus } from '../../types.js';
 import { Text } from 'ink';
 import { StreamingContext } from '../../contexts/StreamingContext.js';
+import { SettingsContext } from '../../contexts/SettingsContext.js';
 import type {
   AnsiOutput,
   AnsiOutputDisplay,
   Config,
 } from '@qwen-code/qwen-code-core';
+import type { LoadedSettings } from '../../../config/settings.js';
 
 vi.mock('../TerminalOutput.js', () => ({
   TerminalOutput: function MockTerminalOutput({
@@ -58,10 +60,17 @@ vi.mock('../GeminiRespondingSpinner.js', () => ({
 vi.mock('./DiffRenderer.js', () => ({
   DiffRenderer: function MockDiffRenderer({
     diffContent,
+    settings,
   }: {
     diffContent: string;
+    settings?: unknown;
   }) {
-    return <Text>MockDiff:{diffContent}</Text>;
+    return (
+      <Text>
+        MockDiff:{diffContent}
+        {settings ? ':withSettings' : ''}
+      </Text>
+    );
   },
 }));
 vi.mock('../../utils/MarkdownDisplay.js', () => ({
@@ -83,6 +92,15 @@ vi.mock('../subagents/index.js', () => ({
   },
 }));
 
+// Mock settings
+const mockSettings: LoadedSettings = {
+  merged: {
+    ui: {
+      showLineNumbers: true,
+    },
+  },
+} as LoadedSettings;
+
 // Helper to render with context
 const renderWithContext = (
   ui: React.ReactElement,
@@ -90,9 +108,11 @@ const renderWithContext = (
 ) => {
   const contextValue: StreamingState = streamingState;
   return render(
-    <StreamingContext.Provider value={contextValue}>
-      {ui}
-    </StreamingContext.Provider>,
+    <SettingsContext.Provider value={mockSettings}>
+      <StreamingContext.Provider value={contextValue}>
+        {ui}
+      </StreamingContext.Provider>
+    </SettingsContext.Provider>,
   );
 };
 

packages/cli/src/ui/components/messages/ToolMessage.tsx

@@ -30,6 +30,8 @@ import {
   TOOL_STATUS,
 } from '../../constants.js';
 import { theme } from '../../semantic-colors.js';
+import { useSettings } from '../../contexts/SettingsContext.js';
+import type { LoadedSettings } from '../../../config/settings.js';
 
 const STATIC_HEIGHT = 1;
 const RESERVED_LINE_COUNT = 5; // for tool name, status, padding etc.
@@ -210,12 +212,14 @@ const DiffResultRenderer: React.FC<{
   data: { fileDiff: string; fileName: string };
   availableHeight?: number;
   childWidth: number;
-}> = ({ data, availableHeight, childWidth }) => (
+  settings?: LoadedSettings;
+}> = ({ data, availableHeight, childWidth, settings }) => (
   <DiffRenderer
     diffContent={data.fileDiff}
     filename={data.fileName}
     availableTerminalHeight={availableHeight}
     contentWidth={childWidth}
+    settings={settings}
   />
 );
 
@@ -243,6 +247,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
   ptyId,
   config,
 }) => {
+  const settings = useSettings();
   const isThisShellFocused =
     (name === SHELL_COMMAND_NAME || name === 'Shell') &&
     status === ToolCallStatus.Executing &&
@@ -348,6 +353,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
                 data={displayRenderer.data}
                 availableHeight={availableHeight}
                 childWidth={innerWidth}
+                settings={settings}
               />
             )}
             {displayRenderer.type === 'ansi' && (