Skip to content

Commit 6bcc512

Browse files
committed
fix: prevent raw tab characters from rendering in terminal input
Tab characters were being rendered as control characters (^I or similar) in the terminal instead of being converted to spaces for display. This caused visual artifacts like "\t▍" instead of showing just the cursor symbol. Changes: - Convert tabs to 4 spaces for rendering (displayValueForRendering) - Calculate cursor position accounting for tab expansion (renderCursorPosition) - Prevent highlighting tab characters (add tab check to shouldHighlight) - Add comprehensive tests for tab rendering behavior The actual stored value still uses \t characters, only the display is affected.
1 parent 79d3f5d commit 6bcc512

File tree

2 files changed

+187
-11
lines changed

2 files changed

+187
-11
lines changed
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
import { describe, test, expect } from 'bun:test'
2+
3+
/**
4+
* Tests for tab character cursor rendering in MultilineInput component.
5+
*
6+
* The shouldHighlight logic determines whether to show a highlighted character
7+
* or the cursor symbol (▍) at the cursor position.
8+
*
9+
* Additionally, tabs are expanded to spaces (TAB_WIDTH=4) for proper rendering,
10+
* so the cursor appears at the correct visual position.
11+
*/
12+
13+
describe('MultilineInput - tab character handling', () => {
14+
const TAB_WIDTH = 4
15+
16+
/**
17+
* Helper function that mimics the shouldHighlight logic from MultilineInput.
18+
* This tests the core fix: tabs should NOT be highlighted (like newlines).
19+
*/
20+
function shouldHighlightChar(
21+
showCursor: boolean,
22+
isPlaceholder: boolean,
23+
cursorPosition: number,
24+
displayValue: string,
25+
): boolean {
26+
return (
27+
showCursor &&
28+
!isPlaceholder &&
29+
cursorPosition < displayValue.length &&
30+
displayValue[cursorPosition] !== '\n' &&
31+
displayValue[cursorPosition] !== '\t' // This is the fix being tested
32+
)
33+
}
34+
35+
/**
36+
* Calculate cursor position in expanded string (tabs -> spaces)
37+
*/
38+
function calculateRenderCursorPosition(
39+
cursorPosition: number,
40+
displayValue: string,
41+
): number {
42+
let renderPos = 0
43+
for (let i = 0; i < cursorPosition && i < displayValue.length; i++) {
44+
renderPos += displayValue[i] === '\t' ? TAB_WIDTH : 1
45+
}
46+
return renderPos
47+
}
48+
49+
test('does NOT highlight when cursor is on a tab character', () => {
50+
const value = 'hello\tworld'
51+
const cursorPosition = 5 // Position of the tab
52+
53+
const shouldHighlight = shouldHighlightChar(true, false, cursorPosition, value)
54+
55+
// Tab characters should not be highlighted (should show cursor symbol instead)
56+
expect(shouldHighlight).toBe(false)
57+
})
58+
59+
test('does NOT highlight when cursor is on a newline character', () => {
60+
const value = 'line1\nline2'
61+
const cursorPosition = 5 // Position of the newline
62+
63+
const shouldHighlight = shouldHighlightChar(true, false, cursorPosition, value)
64+
65+
// Newlines should not be highlighted (existing behavior)
66+
expect(shouldHighlight).toBe(false)
67+
})
68+
69+
test('DOES highlight when cursor is on a regular character', () => {
70+
const value = 'hello'
71+
const cursorPosition = 1 // Position of 'e'
72+
73+
const shouldHighlight = shouldHighlightChar(true, false, cursorPosition, value)
74+
75+
// Regular characters should be highlighted
76+
expect(shouldHighlight).toBe(true)
77+
})
78+
79+
test('does NOT highlight when not focused (showCursor=false)', () => {
80+
const value = 'hello\tworld'
81+
const cursorPosition = 5
82+
83+
const shouldHighlight = shouldHighlightChar(false, false, cursorPosition, value)
84+
85+
expect(shouldHighlight).toBe(false)
86+
})
87+
88+
test('does NOT highlight when showing placeholder', () => {
89+
const value = ''
90+
const cursorPosition = 0
91+
92+
const shouldHighlight = shouldHighlightChar(true, true, cursorPosition, value)
93+
94+
expect(shouldHighlight).toBe(false)
95+
})
96+
97+
test('does NOT highlight when cursor is at end of string', () => {
98+
const value = 'hello'
99+
const cursorPosition = 5 // Beyond last character
100+
101+
const shouldHighlight = shouldHighlightChar(true, false, cursorPosition, value)
102+
103+
expect(shouldHighlight).toBe(false)
104+
})
105+
106+
test('handles multiple tabs - does NOT highlight tab at position 2', () => {
107+
const value = '\t\t\tindented'
108+
const cursorPosition = 2 // Third tab
109+
110+
const shouldHighlight = shouldHighlightChar(true, false, cursorPosition, value)
111+
112+
expect(shouldHighlight).toBe(false)
113+
})
114+
115+
test('handles tab at end of string', () => {
116+
const value = 'text\t'
117+
const cursorPosition = 4 // Position of trailing tab
118+
119+
const shouldHighlight = shouldHighlightChar(true, false, cursorPosition, value)
120+
121+
expect(shouldHighlight).toBe(false)
122+
})
123+
124+
test('handles space character - DOES highlight (spaces are visible)', () => {
125+
const value = 'hello world'
126+
const cursorPosition = 5 // Position of space
127+
128+
const shouldHighlight = shouldHighlightChar(true, false, cursorPosition, value)
129+
130+
// Spaces should be highlighted (they are visible characters)
131+
expect(shouldHighlight).toBe(true)
132+
})
133+
134+
test('expands single tab to 4 spaces for rendering', () => {
135+
const value = 'hello\tworld'
136+
const cursorPosition = 6 // After the tab
137+
138+
const renderPos = calculateRenderCursorPosition(cursorPosition, value)
139+
140+
// Position 6 in original = position 9 in rendered (5 chars + 4-space tab)
141+
expect(renderPos).toBe(9)
142+
})
143+
144+
test('expands multiple tabs correctly', () => {
145+
const value = '\t\t\ttest'
146+
const cursorPosition = 3 // After 3 tabs
147+
148+
const renderPos = calculateRenderCursorPosition(cursorPosition, value)
149+
150+
// 3 tabs = 12 spaces
151+
expect(renderPos).toBe(12)
152+
})
153+
154+
test('mixed content with tabs calculates correct render position', () => {
155+
const value = 'a\tb\tc'
156+
const cursorPosition = 3 // After 'a', tab, 'b'
157+
158+
const renderPos = calculateRenderCursorPosition(cursorPosition, value)
159+
160+
// 'a' (1) + tab (4) + 'b' (1) = 6
161+
expect(renderPos).toBe(6)
162+
})
163+
})

cli/src/components/multiline-input.tsx

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ function findNextWordBoundary(text: string, cursor: number): number {
7676
}
7777

7878
const CURSOR_CHAR = '▍'
79-
const CONTROL_CHAR_REGEX = /[\u0000-\u001f\u007f]/
79+
const CONTROL_CHAR_REGEX = /[\u0000-\u0008\u000b-\u000c\u000e-\u001f\u007f]/
8080

8181
type KeyWithPreventDefault =
8282
| {
@@ -266,14 +266,27 @@ export const MultilineInput = forwardRef<
266266
const isPlaceholder = value.length === 0 && placeholder.length > 0
267267
const displayValue = isPlaceholder ? placeholder : value
268268
const showCursor = focused
269-
const beforeCursor = showCursor ? displayValue.slice(0, cursorPosition) : ''
270-
const afterCursor = showCursor ? displayValue.slice(cursorPosition) : ''
269+
270+
// Replace tabs with spaces for proper rendering
271+
// Terminal tab stops are typically 8 columns, but 4 is more readable
272+
const TAB_WIDTH = 4
273+
const displayValueForRendering = displayValue.replace(/\t/g, ' '.repeat(TAB_WIDTH))
274+
275+
// Calculate cursor position in the expanded string (accounting for tabs)
276+
let renderCursorPosition = 0
277+
for (let i = 0; i < cursorPosition && i < displayValue.length; i++) {
278+
renderCursorPosition += displayValue[i] === '\t' ? TAB_WIDTH : 1
279+
}
280+
281+
const beforeCursor = showCursor ? displayValueForRendering.slice(0, renderCursorPosition) : ''
282+
const afterCursor = showCursor ? displayValueForRendering.slice(renderCursorPosition) : ''
271283
const activeChar = afterCursor.charAt(0) || ' '
272284
const shouldHighlight =
273285
showCursor &&
274286
!isPlaceholder &&
275287
cursorPosition < displayValue.length &&
276-
displayValue[cursorPosition] !== '\n'
288+
displayValue[cursorPosition] !== '\n' &&
289+
displayValue[cursorPosition] !== '\t'
277290

278291
// Handle all keyboard input with advanced shortcuts
279292
useKeyboard(
@@ -739,15 +752,15 @@ export const MultilineInput = forwardRef<
739752

740753
const layoutContent = showCursor
741754
? shouldHighlight
742-
? displayValue
743-
: `${displayValue.slice(0, cursorPosition)}${CURSOR_CHAR}${afterCursor}`
744-
: displayValue
755+
? displayValueForRendering
756+
: `${beforeCursor}${CURSOR_CHAR}${afterCursor}`
757+
: displayValueForRendering
745758

746759
const cursorProbe = showCursor
747760
? shouldHighlight
748-
? displayValue.slice(0, cursorPosition + 1)
749-
: `${displayValue.slice(0, cursorPosition)}${CURSOR_CHAR}`
750-
: displayValue.slice(0, cursorPosition)
761+
? displayValueForRendering.slice(0, renderCursorPosition + 1)
762+
: `${beforeCursor}${CURSOR_CHAR}`
763+
: displayValueForRendering.slice(0, renderCursorPosition)
751764

752765
const layoutMetrics = useMemo(
753766
() =>
@@ -834,7 +847,7 @@ export const MultilineInput = forwardRef<
834847
</>
835848
) : (
836849
<>
837-
{displayValue}
850+
{displayValueForRendering}
838851
{shouldRenderBottomGutter ? '\n' : ''}
839852
</>
840853
)}

0 commit comments

Comments
 (0)