Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pager preview with escape sequence and newlines #1069

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

tompng
Copy link
Member

@tompng tompng commented Jan 21, 2025

Fixes this bug

irb(main):001> 100.times.map{[1]*it}
An error occurred when inspecting the object: #<TypeError: no implicit conversion of nil into String>
Result of Kernel#inspect: #<Array:0x0000000129d94778>
=> 
[[],
 [1],
 [1, 1],
 [1, 1, 1],
 [1, 1, 1, 1],
 [1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
 [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
irb(main):002> 

Non-string

PP doesn't seem to write non-string object, but fix it in case.

text = text.to_s unless text.is_a?(String)

Escape sequence

Need to set the second parameter allow_escape_code to Reline::Unicode.calculate_width

Reline::Unicode.calculate_width("\e[31mA\e[m") #=> 11 because length of "^[[31mA^[[m"(escaped for printing) is 11
Reline::Unicode.calculate_width("\e[31mA\e[m", true) #=> 1

Newline

Reline::Unicode.split_by_width(line.chomp, @width)
wrapped_lines does not include "\n" because line is chomp-ed. (split_by_width does not work if string contains newline character)
So we need to add "\n" if line ends with "\n". And the logic had a bug.
PP doesn't seem to write continuous newlines, but found while adding more tests.

Copy link
Member

@ima1zumi ima1zumi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ima1zumi ima1zumi merged commit a139562 into ruby:master Jan 22, 2025
34 checks passed
@tompng tompng deleted the pager_preview_bugfix branch January 22, 2025 05:09
@tompng tompng mentioned this pull request Jan 22, 2025
@tompng tompng added the bug Something isn't working label Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

2 participants