Skip to content

Fix pager preview with escape sequence and newlines#1069

Merged
ima1zumi merged 1 commit intoruby:masterfrom
tompng:pager_preview_bugfix
Jan 22, 2025
Merged

Fix pager preview with escape sequence and newlines#1069
ima1zumi merged 1 commit intoruby:masterfrom
tompng:pager_preview_bugfix

Conversation

@tompng
Copy link
Member

@tompng tompng commented Jan 21, 2025 •
edited
Loading

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.

ima1zumi
ima1zumi approved these changes Jan 22, 2025
View reviewed changes
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
Merged
tompng added a commit to tompng/ruby that referenced this pull request Jan 22, 2025
@tompng
[ruby/irb] Fix pager preview with escape sequence and newlines
6fd59cd 
(ruby/irb#1069)

ruby/irb@a139562a07
tompng added a commit to tompng/ruby that referenced this pull request Jan 22, 2025
@tompng
[ruby/irb] Fix pager preview with escape sequence and newlines
f9b7a78 
(ruby/irb#1069)

ruby/irb@a139562a07
tompng added a commit to ruby/ruby that referenced this pull request Jan 22, 2025
@tompng
[ruby/irb] Fix pager preview with escape sequence and newlines
c066453 
(ruby/irb#1069)

ruby/irb@a139562a07
@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

Reviewers

@ima1zumi ima1zumi ima1zumi approved these changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tompng @ima1zumi