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

Remove unused things from reline/unicode.rb #759

Merged
merged 4 commits into from
Nov 10, 2024

Conversation

tompng
Copy link
Member

@tompng tompng commented Oct 9, 2024

Remove unused value from Reline::Unicode method's return value

split_by_width

# Before: returns [splitted_lines_including_nil_at_odd_index, height]
Reline::Unicode.split_by_width('0123456789', 3)
#=> [["012", nil, "345", nil, "678", nil, "9"], 4]

# After: height is unused. nil is also unused, always removed with Array#compact
Reline::Unicode.split_line_by_width('0123456789', 3)
#=> ["012", "345", "678", "9"]

split_line_by_width

IRB uses legacy split_by_width. We need to rename it for refactored implementation and add a legacy split_by_width back.

# lib/irb.rb
lines, _ = Reline::Unicode.split_by_width
str = "%s..." % lines.first
# IRB only uses first string in the array. Inserting nil to the array is not necessary.
Reline::Unicode.split_by_width('0123456789', 3)
#=> [["012", "345", "678", "9"], 4]

em_ vi_ methods

Reline::Unicode.em_forward_word Reline::Unicode.vi_big_forward_word and more methods returned [byte_size, visible_width] but visible_width part is unused.

These method didn't had tests. Tests are added to ensure this change didn't break anything.

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.

Amazing! Thank you!

@ima1zumi
Copy link
Member

Could you rebase it?

@ima1zumi
Copy link
Member

Thanks!

@ima1zumi ima1zumi merged commit f32446e into ruby:master Nov 10, 2024
46 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Nov 10, 2024
(ruby/reline#759)

* Remove garbage(nil) from Reline::Unicode.split_by_width result

* Remove unused width from Reline::Unicode vi_ ed_ em_ method return value

* Remove unused height from Unicode.split_by_width return value

* Rename split_by_width to split_line_by_width and add legacy split_by_width for IRB

ruby/reline@f32446ebc4
@tompng tompng deleted the unicode_remove_unused branch November 10, 2024 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants