Skip to content

Commit ed20b1a

Browse files
committed
Fix line length calculation for interactive prompt
1 parent 6bdefd1 commit ed20b1a

2 files changed

Lines changed: 54 additions & 1 deletion

File tree

lib/cli/ui/prompt/interactive_options.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,10 @@ def calculate_option_line_lengths
120120
next 1 if text.empty?
121121

122122
# Find the length of all the lines in this string
123+
# Use ANSI.printing_width to correctly calculate display width,
124+
# stripping invisible escape sequences like OSC 8 hyperlinks
123125
non_empty_line_lengths = "#{prefix}#{text}".split("\n").reject(&:empty?).map do |line|
124-
CLI::UI.fmt(line, enable_color: false).length
126+
CLI::UI::ANSI.printing_width(CLI::UI.fmt(line, enable_color: false))
125127
end
126128

127129
# Finally, we need to calculate how many lines each one will take. We can do that by dividing each one

test/cli/ui/prompt_test.rb

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,57 @@ def test_ask_interactive_line_selection
292292
assert_output_includes('--10--')
293293
end
294294

295+
def test_ask_multiple_with_link_options_uses_display_width_for_cursor_positioning
296+
# This test verifies that line length calculation uses display width, not string length.
297+
# Links have OSC 8 escape sequences (~50 invisible chars) but short display text.
298+
# At terminal width 40:
299+
# - Correct (display width): each option ~15 chars → 1 line each → 4 total lines
300+
# - Buggy (string length): each option ~60 chars → 2 lines each → 7 total lines
301+
# After navigation, reset_position moves cursor up by num_lines.
302+
# We verify the cursor moves up 4 times (correct), not 7 times (buggy).
303+
run_in_process(<<~RUBY)
304+
# Override terminal width to trigger the bug scenario
305+
module CLI::UI::Terminal
306+
def self.width; 40; end
307+
end
308+
309+
options = ['a', 'b', 'c'].map do |x|
310+
CLI::UI.link("https://example.com/\#{x}", "Link \#{x.upcase}")
311+
end
312+
CLI::UI::Prompt.ask('q', options: options, multiple: true)
313+
RUBY
314+
315+
# Wait for initial render, then navigate down
316+
wait_for_output_to_include('Link A')
317+
write('j')
318+
# Navigate triggers redraw - wait for cursor movement sequences
319+
wait_for_output_to_include("\e[1A")
320+
321+
# Complete the prompt so the process exits
322+
write('0')
323+
324+
clean_up do
325+
# Combine the output we already read with any remaining
326+
output = @output.dup.force_encoding('UTF-8') + @stdout.read
327+
328+
# Count cursor-up sequences (\e[1A\e[1G) from navigation
329+
# Each reset_position(n) produces n cursor-up sequences
330+
cursor_up_sequences = output.scan("\e[1A\e[1G")
331+
332+
# Cursor-ups = (num_lines × 3 resets) + 2 from Prompt wrapper
333+
# With correct display-width calculation: 4 lines (3 options + Done)
334+
# → (4 × 3) + 2 = 14 cursor-ups
335+
# With buggy string-length calculation: 7 lines (each link ~2 lines at width 40)
336+
# → (7 × 3) + 2 = 23 cursor-ups
337+
assert_equal(
338+
14,
339+
cursor_up_sequences.length,
340+
'Expected 14 cursor-up sequences for correct display-width line calculation. ' \
341+
"Got #{cursor_up_sequences.length}; if significantly higher, string length may have been used instead of display width.",
342+
)
343+
end
344+
end
345+
295346
def test_ask_multiple
296347
run_in_process('puts CLI::UI::Prompt.ask("q", options: (1..15).map(&:to_s), multiple: true).inspect')
297348
write('1350')

0 commit comments

Comments
 (0)