Skip to content

Fix dialog deleting fullwidth characters #471

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.

Sign up for GitHub

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

Closed
tompng wants to merge 5 commits into from
Closed

Fix dialog deleting fullwidth characters #471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
abc2309
Refactor reset_dialog rerendering
tompng Sep 22, 2022
cb6ebe1
Fix fullwidth characters deleted by dialog
tompng Sep 27, 2022
f715f01
Fix inconsistency of take_range and calculate_width for string with N…
tompng Sep 27, 2022
4b6ad70
Add test for take_range options
tompng Sep 27, 2022
7a07a1a
Add rerender fullwidth chars under dialog test
tompng Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 34 additions & 85 deletions lib/reline/line_editor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,13 +654,6 @@ def add_dialog_proc(name, p, context = nil)
end
end

private def padding_space_with_escape_sequences(str, width)
padding_width = width - calculate_width(str, true)
# padding_width should be only positive value. But macOS and Alacritty returns negative value.
padding_width = 0 if padding_width < 0
str + (' ' * padding_width)
end

private def render_each_dialog(dialog, cursor_column)
if @in_pasting
clear_each_dialog(dialog)
Expand Down Expand Up @@ -753,7 +746,7 @@ def add_dialog_proc(name, p, context = nil)
bg_color = dialog_render_info.bg_color
end
str_width = dialog.width - (dialog.scrollbar_pos.nil? ? 0 : @block_elem_width)
str = padding_space_with_escape_sequences(Reline::Unicode.take_range(item, 0, str_width), str_width)
str, = Reline::Unicode.take_range(item, 0, str_width, padding: true)
@output.write "\e[#{bg_color}m\e[#{fg_color}m#{str}"
if dialog.scrollbar_pos and (dialog.scrollbar_pos != old_dialog.scrollbar_pos or dialog.column != old_dialog.column)
@output.write "\e[37m"
Expand Down Expand Up @@ -797,85 +790,42 @@ def add_dialog_proc(name, p, context = nil)
end
visual_lines.concat(vl)
}
old_y = dialog.lines_backup[:first_line_started_from] + dialog.lines_backup[:started_from]
y = @first_line_started_from + @started_from
y_diff = y - old_y
if (old_y + old_dialog.vertical_offset) < (y + dialog.vertical_offset)
# rerender top
move_cursor_down(old_dialog.vertical_offset - y_diff)
start = visual_start + old_dialog.vertical_offset
line_num = dialog.vertical_offset - old_dialog.vertical_offset
line_num.times do |i|
Reline::IOGate.move_cursor_column(old_dialog.column)
if visual_lines[start + i].nil?
s = ' ' * old_dialog.width
else
s = Reline::Unicode.take_range(visual_lines[start + i], old_dialog.column, old_dialog.width)
s = padding_space_with_escape_sequences(s, old_dialog.width)
end
@output.write "\e[0m#{s}\e[0m"
move_cursor_down(1) if i < (line_num - 1)
end
move_cursor_up(old_dialog.vertical_offset + line_num - 1 - y_diff)
end
if (old_y + old_dialog.vertical_offset + old_dialog.contents.size) > (y + dialog.vertical_offset + dialog.contents.size)
# rerender bottom
move_cursor_down(dialog.vertical_offset + dialog.contents.size - y_diff)
start = visual_start + dialog.vertical_offset + dialog.contents.size
line_num = (old_dialog.vertical_offset + old_dialog.contents.size) - (dialog.vertical_offset + dialog.contents.size)
line_num.times do |i|
Reline::IOGate.move_cursor_column(old_dialog.column)
if visual_lines[start + i].nil?
s = ' ' * old_dialog.width
else
s = Reline::Unicode.take_range(visual_lines[start + i], old_dialog.column, old_dialog.width)
s = padding_space_with_escape_sequences(s, old_dialog.width)
old_dialog_y = dialog.lines_backup[:first_line_started_from] + dialog.lines_backup[:started_from]
dialog_y = @first_line_started_from + @started_from

x_range = dialog.column...dialog.column + dialog.width
old_x_range = old_dialog.column...old_dialog.column + old_dialog.width
y_range = dialog_y + dialog.vertical_offset...dialog_y + dialog.vertical_offset + dialog.contents.size
old_y_range = old_dialog_y + old_dialog.vertical_offset...old_dialog_y + old_dialog.vertical_offset + old_dialog.contents.size

cursor_y = dialog_y
old_y_range.each do |y|
rerender_ranges = []
if y_range.cover?(y) && x_range.any?(old_x_range)
if old_x_range.begin < x_range.begin
# rerender left
rerender_ranges << [old_x_range.begin...[old_x_range.end, x_range.begin].min, true, false]
end
@output.write "\e[0m#{s}\e[0m"
move_cursor_down(1) if i < (line_num - 1)
end
move_cursor_up(dialog.vertical_offset + dialog.contents.size + line_num - 1 - y_diff)
end
if old_dialog.column < dialog.column
# rerender left
move_cursor_down(old_dialog.vertical_offset - y_diff)
width = dialog.column - old_dialog.column
start = visual_start + old_dialog.vertical_offset
line_num = old_dialog.contents.size
line_num.times do |i|
Reline::IOGate.move_cursor_column(old_dialog.column)
if visual_lines[start + i].nil?
s = ' ' * width
else
s = Reline::Unicode.take_range(visual_lines[start + i], old_dialog.column, width)
s = padding_space_with_escape_sequences(s, dialog.width)
if x_range.end < old_x_range.end
# rerender right
rerender_ranges << [[x_range.end, old_x_range.begin].max...old_x_range.end, false, true]
end
else
rerender_ranges << [old_x_range, true, true]
end

rerender_ranges.each do |range, cover_begin, cover_end|
move_cursor_down(y - cursor_y)
cursor_y = y
col = range.begin
width = range.end - range.begin
line = visual_lines[y + visual_start - old_dialog_y] || ''
s, col = Reline::Unicode.take_range(line, col, width, cover_begin: cover_begin, cover_end: cover_end, padding: true)
Reline::IOGate.move_cursor_column(col)
@output.write "\e[0m#{s}\e[0m"
move_cursor_down(1) if i < (line_num - 1)
end
move_cursor_up(old_dialog.vertical_offset + line_num - 1 - y_diff)
end
if (old_dialog.column + old_dialog.width) > (dialog.column + dialog.width)
# rerender right
move_cursor_down(old_dialog.vertical_offset + y_diff)
width = (old_dialog.column + old_dialog.width) - (dialog.column + dialog.width)
start = visual_start + old_dialog.vertical_offset
line_num = old_dialog.contents.size
line_num.times do |i|
Reline::IOGate.move_cursor_column(old_dialog.column + dialog.width)
if visual_lines[start + i].nil?
s = ' ' * width
else
s = Reline::Unicode.take_range(visual_lines[start + i], old_dialog.column + dialog.width, width)
rerender_width = old_dialog.width - dialog.width
s = padding_space_with_escape_sequences(s, rerender_width)
end
Reline::IOGate.move_cursor_column(dialog.column + dialog.width)
@output.write "\e[0m#{s}\e[0m"
move_cursor_down(1) if i < (line_num - 1)
end
move_cursor_up(old_dialog.vertical_offset + line_num - 1 + y_diff)
end
move_cursor_up(cursor_y - dialog_y)
Reline::IOGate.move_cursor_column((prompt_width + @cursor) % @screen_size.last)
end

Expand Down Expand Up @@ -916,9 +866,8 @@ def add_dialog_proc(name, p, context = nil)
dialog_vertical_size = dialog.contents.size
dialog_vertical_size.times do |i|
if i < visual_lines_under_dialog.size
Reline::IOGate.move_cursor_column(dialog.column)
str = Reline::Unicode.take_range(visual_lines_under_dialog[i], dialog.column, dialog.width)
str = padding_space_with_escape_sequences(str, dialog.width)
str, start_pos = Reline::Unicode.take_range(visual_lines_under_dialog[i], dialog.column, dialog.width, cover_begin: true, cover_end: true, padding: true)
Reline::IOGate.move_cursor_column(start_pos)
@output.write "\e[0m#{str}\e[0m"
else
Reline::IOGate.move_cursor_column(dialog.column)
Expand Down
30 changes: 26 additions & 4 deletions lib/reline/unicode.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,17 +194,21 @@ def self.split_by_width(str, max_width, encoding = str.encoding)
end

# Take a chunk of a String cut by width with escape sequences.
def self.take_range(str, start_col, max_width, encoding = str.encoding)
def self.take_range(str, start_col, width, encoding: str.encoding, cover_begin: false, cover_end: false, padding: false)
chunk = String.new(encoding: encoding)
total_width = 0
rest = str.encode(Encoding::UTF_8)
in_zero_width = false
chunk_start_col = nil
chunk_end_col = nil
rest.scan(WIDTH_SCANNER) do |gc|
case
when gc[NON_PRINTING_START_INDEX]
in_zero_width = true
chunk << NON_PRINTING_START
when gc[NON_PRINTING_END_INDEX]
in_zero_width = false
chunk << NON_PRINTING_END
when gc[CSI_REGEXP_INDEX]
chunk << gc[CSI_REGEXP_INDEX]
when gc[OSC_REGEXP_INDEX]
Expand All @@ -215,13 +219,31 @@ def self.take_range(str, start_col, max_width, encoding = str.encoding)
chunk << gc
else
mbchar_width = get_mbchar_width(gc)
prev_width = total_width
total_width += mbchar_width
break if (start_col + max_width) < total_width
chunk << gc if start_col < total_width
break if !cover_end && total_width > start_col + width
if cover_begin ? start_col < total_width : start_col <= prev_width
chunk << gc
chunk_start_col ||= prev_width
chunk_end_col = total_width
end
break if total_width >= start_col + width
end
end
end
chunk
chunk_start_col ||= start_col
chunk_end_col ||= start_col
if padding
if start_col < chunk_start_col
chunk = ' ' * (chunk_start_col - start_col) + chunk
chunk_start_col = start_col
end
if chunk_end_col < start_col + width
chunk << ' ' * (start_col + width - chunk_end_col)
chunk_end_col = start_col + width
end
end
[chunk, chunk_start_col, chunk_end_col - chunk_start_col]
end

def self.get_next_mbchar_size(line, byte_pointer)
Expand Down
18 changes: 16 additions & 2 deletions test/reline/test_unicode.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,21 @@ def test_ambiguous_width
end

def test_take_range
assert_equal 'cdef', Reline::Unicode.take_range('abcdefghi', 2, 4)
assert_equal 'いう', Reline::Unicode.take_range('あいうえお', 2, 4)
assert_equal ['cdef', 2, 4], Reline::Unicode.take_range('abcdefghi', 2, 4)
assert_equal ['cdef', 2, 4], Reline::Unicode.take_range('abcdefghi', 2, 4, padding: true)
assert_equal ['cdef', 2, 4], Reline::Unicode.take_range('abcdefghi', 2, 4, cover_begin: true)
assert_equal ['cdef', 2, 4], Reline::Unicode.take_range('abcdefghi', 2, 4, cover_end: true)
assert_equal ['いう', 2, 4], Reline::Unicode.take_range('あいうえお', 2, 4)
assert_equal ['いう', 2, 4], Reline::Unicode.take_range('あいうえお', 2, 4, padding: true)
assert_equal ['いう', 2, 4], Reline::Unicode.take_range('あいうえお', 2, 4, cover_begin: true)
assert_equal ['いう', 2, 4], Reline::Unicode.take_range('あいうえお', 2, 4, cover_end: true)
assert_equal ['う', 4, 2], Reline::Unicode.take_range('あいうえお', 3, 4)
assert_equal [' う ', 3, 4], Reline::Unicode.take_range('あいうえお', 3, 4, padding: true)
assert_equal ['いう', 2, 4], Reline::Unicode.take_range('あいうえお', 3, 4, cover_begin: true)
assert_equal ['うえ', 4, 4], Reline::Unicode.take_range('あいうえお', 3, 4, cover_end: true)
assert_equal ['いう ', 2, 5], Reline::Unicode.take_range('あいうえお', 3, 4, cover_begin: true, padding: true)
assert_equal [' うえ', 3, 5], Reline::Unicode.take_range('あいうえお', 3, 4, cover_end: true, padding: true)
assert_equal [' うえお ', 3, 10], Reline::Unicode.take_range('あいうえお', 3, 10, padding: true)
assert_equal ["\e[31mc\1ABC\2d\e[0mef", 2, 4], Reline::Unicode.take_range("\e[31mabc\1ABC\2d\e[0mefghi", 2, 4)
end
end
16 changes: 16 additions & 0 deletions test/reline/yamatanooroti/test_rendering.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,22 @@ def test_autocomplete_rerender_under_dialog
EOC
end

def test_autocomplete_rerender_fullwidth_under_dialog
start_terminal(20, 40, %W{ruby -I#{@pwd}/lib #{@pwd}/test/reline/yamatanooroti/multiline_repl --autocomplete}, startup_message: 'Multiline REPL.')
write("def hoge\n\n あいうえおかきくけこab\n aあいうえおかきくけこb\n abあいうえおかきくけこ\C-p\C-p\C-p ")
write('S')
write('t')
close
assert_screen(<<~'EOC')
Multiline REPL.
prompt> def hoge
prompt> St
prompt> Stringえおかきくけこab
prompt> Struct えおかきくけこb
prompt> abあいうえおかきくけこ
EOC
end

def test_autocomplete_long_with_scrollbar
start_terminal(20, 30, %W{ruby -I#{@pwd}/lib #{@pwd}/test/reline/yamatanooroti/multiline_repl --autocomplete-long}, startup_message: 'Multiline REPL.')
write('S')
Expand Down