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.

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

Jump to bottom

Refactor waiting_proc and waiting_operator_proc #649

Merged
merged 7 commits into from
Apr 14, 2024
Merged

Refactor waiting_proc and waiting_operator_proc #649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
186a1e4
Fix waiting_proc precedence
tompng Dec 30, 2023
9f157a9
Fix waiting_operator bugs
tompng Dec 30, 2023
07adea0
Add waiting_proc and vi_waiting_operator test
tompng Dec 31, 2023
20b5e71
Fix vi waiting operator arg number
tompng Apr 8, 2024
02e54fe
Implement `yy` copies whole line in vi_command mode
tompng Apr 8, 2024
3125878
Simplify incremental search cancel test
tompng Apr 8, 2024
4e733ab
Add complex vi test with waiting_proc and vi_waiting_operator, split …
tompng Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
151 changes: 85 additions & 66 deletions lib/reline/line_editor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ def reset_variables(prompt = '', encoding:)
@vi_clipboard = ''
@vi_arg = nil
@waiting_proc = nil
@waiting_operator_proc = nil
@waiting_operator_vi_arg = nil
@vi_waiting_operator = nil
@vi_waiting_operator_arg = nil
@completion_journey_state = nil
@completion_state = CompletionState::NORMAL
@perfect_matched = nil
Expand Down Expand Up @@ -935,37 +935,23 @@ def dialog_proc_scope_completion_journey_data
end

private def run_for_operators(key, method_symbol, &block)
if @waiting_operator_proc
if @vi_waiting_operator
if VI_MOTIONS.include?(method_symbol)
old_byte_pointer = @byte_pointer
@vi_arg = @waiting_operator_vi_arg if @waiting_operator_vi_arg&.> 1
@vi_arg = (@vi_arg || 1) * @vi_waiting_operator_arg
block.(true)
unless @waiting_proc
byte_pointer_diff = @byte_pointer - old_byte_pointer
@byte_pointer = old_byte_pointer
@waiting_operator_proc.(byte_pointer_diff)
else
old_waiting_proc = @waiting_proc
old_waiting_operator_proc = @waiting_operator_proc
current_waiting_operator_proc = @waiting_operator_proc
@waiting_proc = proc { |k|
old_byte_pointer = @byte_pointer
old_waiting_proc.(k)
byte_pointer_diff = @byte_pointer - old_byte_pointer
@byte_pointer = old_byte_pointer
current_waiting_operator_proc.(byte_pointer_diff)
@waiting_operator_proc = old_waiting_operator_proc
}
send(@vi_waiting_operator, byte_pointer_diff)
Copy link
Member

Choose a reason for hiding this comment

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

For my learning: Why pass byte_pointer_diff? Are multi-byte characters allowed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. multi-byte characters are allowed.

vi_waiting_operator will do

  1. save cursor position (represented by @byte_pointer)
  2. accept VI_MOTIONS. cursor position moves. (@line_index does not change)
  3. Run an action(==@vi_waiting_operator) to a text between old cursor position and new cursor position

Current cursor position can be retrieved from @byte_pointer, so we need to pass old cursor_position or difference between old and new curspor_position
The original implementation selected diff, and this pull request follows it.

And we can choose how to represent cursor position: byte_pointer, string size, grapheme_cluster size width in column when rendered to terminal. Using byte_position is easiest because we don't need to translate @byte_pointer to another representation form.

Copy link
Member

Choose a reason for hiding this comment

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

I understood. Thanks for explaining!

cleanup_waiting
end
else
# Ignores operator when not motion is given.
block.(false)
cleanup_waiting
end
@waiting_operator_proc = nil
@waiting_operator_vi_arg = nil
if @vi_arg
@vi_arg = nil
end
@vi_arg = nil
Copy link
Member

Choose a reason for hiding this comment

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

For my learning: So all we need to do here is initialize vi_arg, why don't we call cleanup_waiting?

Copy link
Member Author

Choose a reason for hiding this comment

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

cleanup_waiting is to clear @waiting_proc relevant things. @waiting_proc is a mechanism to override input_key.
For example, C-h is normally handled by em_delete_prev_char, but in incremental search mode, it is handled by waiting_proc.

On the other hand, @vi_arg is not related to waiting_proc mechanism. It is passed to other normal ed_xxx vi_xxx em_xxx methods as a keyword parameter.

Copy link
Member Author

@tompng tompng Apr 11, 2024 •
edited

Choose a reason for hiding this comment

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

Example: 2d3fo will do dfo (delete to next char o) 6 times,

2: ed_argument_digit sets @vi_arg = 2
d: vi_delete_meta sets @vi_waiting_operator, @vi_waiting_operator_arg = :vi_delete_meta_confirm, 2
3: ed_argument_digit sets @vi_arg = 3
f: vi_next_char sets @waiting_proc. run_for_operators should skips calling cleanup_waiting when @waiting_proc is set.
o: @vi_arg is set to 6, @waiting_proc is called (cursor moves), @vi_waiting_operator deletes text between cursor movement.

(I added this to test case)

Copy link
Member

Choose a reason for hiding this comment

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

I misunderstood the relationship between @waiting_operator and @vi_waiting_operator. Thanks!

else
block.(false)
end
Expand All @@ -982,7 +968,7 @@ def dialog_proc_scope_completion_journey_data
end

def wrap_method_call(method_symbol, method_obj, key, with_operator = false)
if @config.editing_mode_is?(:emacs, :vi_insert) and @waiting_proc.nil? and @waiting_operator_proc.nil?
if @config.editing_mode_is?(:emacs, :vi_insert) and @vi_waiting_operator.nil?
not_insertion = method_symbol != :ed_insert
process_insert(force: not_insertion)
end
Expand All @@ -1001,11 +987,32 @@ def wrap_method_call(method_symbol, method_obj, key, with_operator = false)
end
end

private def cleanup_waiting
@waiting_proc = nil
@vi_waiting_operator = nil
@vi_waiting_operator_arg = nil
@searching_prompt = nil
@drop_terminate_spaces = false
end

private def process_key(key, method_symbol)
if key.is_a?(Symbol)
cleanup_waiting
elsif @waiting_proc
old_byte_pointer = @byte_pointer
@waiting_proc.call(key)
if @vi_waiting_operator
byte_pointer_diff = @byte_pointer - old_byte_pointer
@byte_pointer = old_byte_pointer
send(@vi_waiting_operator, byte_pointer_diff)
cleanup_waiting
end
@kill_ring.process
return
end

if method_symbol and respond_to?(method_symbol, true)
method_obj = method(method_symbol)
else
method_obj = nil
end
if method_symbol and key.is_a?(Symbol)
if @vi_arg and argumentable?(method_obj)
Expand All @@ -1027,8 +1034,6 @@ def wrap_method_call(method_symbol, method_obj, key, with_operator = false)
run_for_operators(key, method_symbol) do |with_operator|
wrap_method_call(method_symbol, method_obj, key, with_operator)
end
elsif @waiting_proc
@waiting_proc.(key)
elsif method_obj
wrap_method_call(method_symbol, method_obj, key)
else
Expand All @@ -1039,9 +1044,6 @@ def wrap_method_call(method_symbol, method_obj, key, with_operator = false)
@vi_arg = nil
end
end
elsif @waiting_proc
@waiting_proc.(key)
@kill_ring.process
elsif method_obj
if method_symbol == :ed_argument_digit
wrap_method_call(method_symbol, method_obj, key)
Expand Down Expand Up @@ -2318,46 +2320,63 @@ def finish
@byte_pointer = 0
end

private def vi_change_meta(key, arg: 1)
@drop_terminate_spaces = true
@waiting_operator_proc = proc { |byte_pointer_diff|
if byte_pointer_diff > 0
line, cut = byteslice!(current_line, @byte_pointer, byte_pointer_diff)
elsif byte_pointer_diff < 0
line, cut = byteslice!(current_line, @byte_pointer + byte_pointer_diff, -byte_pointer_diff)
end
set_current_line(line)
copy_for_vi(cut)
@byte_pointer += byte_pointer_diff if byte_pointer_diff < 0
@config.editing_mode = :vi_insert
@drop_terminate_spaces = false
}
@waiting_operator_vi_arg = arg
private def vi_change_meta(key, arg: nil)
if @vi_waiting_operator
set_current_line('', 0) if @vi_waiting_operator == :vi_change_meta_confirm && arg.nil?
@vi_waiting_operator = nil
@vi_waiting_operator_arg = nil
else
@drop_terminate_spaces = true
@vi_waiting_operator = :vi_change_meta_confirm
@vi_waiting_operator_arg = arg || 1
end
end

private def vi_delete_meta(key, arg: 1)
@waiting_operator_proc = proc { |byte_pointer_diff|
if byte_pointer_diff > 0
line, cut = byteslice!(current_line, @byte_pointer, byte_pointer_diff)
elsif byte_pointer_diff < 0
line, cut = byteslice!(current_line, @byte_pointer + byte_pointer_diff, -byte_pointer_diff)
end
copy_for_vi(cut)
set_current_line(line || '', @byte_pointer + (byte_pointer_diff < 0 ? byte_pointer_diff : 0))
}
@waiting_operator_vi_arg = arg
private def vi_change_meta_confirm(byte_pointer_diff)
vi_delete_meta_confirm(byte_pointer_diff)
@config.editing_mode = :vi_insert
@drop_terminate_spaces = false
end

private def vi_yank(key, arg: 1)
@waiting_operator_proc = proc { |byte_pointer_diff|
if byte_pointer_diff > 0
cut = current_line.byteslice(@byte_pointer, byte_pointer_diff)
elsif byte_pointer_diff < 0
cut = current_line.byteslice(@byte_pointer + byte_pointer_diff, -byte_pointer_diff)
end
copy_for_vi(cut)
}
@waiting_operator_vi_arg = arg
private def vi_delete_meta(key, arg: nil)
if @vi_waiting_operator
set_current_line('', 0) if @vi_waiting_operator == :vi_delete_meta_confirm && arg.nil?
@vi_waiting_operator = nil
@vi_waiting_operator_arg = nil
else
@vi_waiting_operator = :vi_delete_meta_confirm
@vi_waiting_operator_arg = arg || 1
end
end

private def vi_delete_meta_confirm(byte_pointer_diff)
if byte_pointer_diff > 0
line, cut = byteslice!(current_line, @byte_pointer, byte_pointer_diff)
elsif byte_pointer_diff < 0
line, cut = byteslice!(current_line, @byte_pointer + byte_pointer_diff, -byte_pointer_diff)
end
copy_for_vi(cut)
set_current_line(line || '', @byte_pointer + (byte_pointer_diff < 0 ? byte_pointer_diff : 0))
end

private def vi_yank(key, arg: nil)
if @vi_waiting_operator
copy_for_vi(current_line) if @vi_waiting_operator == :vi_yank_confirm && arg.nil?
@vi_waiting_operator = nil
@vi_waiting_operator_arg = nil
else
@vi_waiting_operator = :vi_yank_confirm
@vi_waiting_operator_arg = arg || 1
end
end

private def vi_yank_confirm(byte_pointer_diff)
if byte_pointer_diff > 0
cut = current_line.byteslice(@byte_pointer, byte_pointer_diff)
elsif byte_pointer_diff < 0
cut = current_line.byteslice(@byte_pointer + byte_pointer_diff, -byte_pointer_diff)
end
copy_for_vi(cut)
end

private def vi_list_or_eof(key)
Expand Down
8 changes: 8 additions & 0 deletions test/reline/test_key_actor_emacs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1263,6 +1263,14 @@ def test_ed_search_next_history_with_empty
assert_line_around_cursor('', '')
end

def test_incremental_search_history_cancel_by_symbol_key
# ed_prev_char should move cursor left and cancel incremental search
input_keys("abc\C-r")
input_key_by_symbol(:ed_prev_char)
input_keys('d')
assert_line_around_cursor('abd', 'c')
end

# Unicode emoji test
def test_ed_insert_for_include_zwj_emoji
omit "This test is for UTF-8 but the locale is #{Reline.core.encoding}" if Reline.core.encoding != Encoding::UTF_8
Expand Down
63 changes: 56 additions & 7 deletions test/reline/test_key_actor_vi.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,10 +693,16 @@ def test_vi_delete_meta_with_vi_next_char
end

def test_vi_delete_meta_with_arg
input_keys("aaa bbb ccc\C-[02w")
assert_line_around_cursor('aaa bbb ', 'ccc')
input_keys("aaa bbb ccc ddd\C-[03w")
assert_line_around_cursor('aaa bbb ccc ', 'ddd')
input_keys('2dl')
assert_line_around_cursor('aaa bbb ', 'c')
assert_line_around_cursor('aaa bbb ccc ', 'd')
input_keys('d2h')
assert_line_around_cursor('aaa bbb cc', 'd')
input_keys('2d3h')
assert_line_around_cursor('aaa ', 'd')
input_keys('dd')
assert_line_around_cursor('', '')
end

def test_vi_change_meta
Expand All @@ -719,6 +725,45 @@ def test_vi_change_meta_with_vi_next_word
assert_line_around_cursor('foo hog', 'e baz')
end

def test_vi_waiting_operator_with_waiting_proc
input_keys("foo foo foo foo foo\C-[0")
input_keys('2d3fo')
assert_line_around_cursor('', ' foo foo')
input_keys('fo')
assert_line_around_cursor(' f', 'oo foo')
end

def test_vi_waiting_operator_cancel
input_keys("aaa bbb ccc\C-[02w")
assert_line_around_cursor('aaa bbb ', 'ccc')
# dc dy should cancel delete_meta
input_keys('dch')
input_keys('dyh')
# cd cy should cancel change_meta
input_keys('cdh')
input_keys('cyh')
# yd yc should cancel yank_meta
# P should not paste yanked text because yank_meta is canceled
input_keys('ydhP')
input_keys('ychP')
assert_line_around_cursor('aa', 'a bbb ccc')
end

def test_cancel_waiting_with_symbol_key
input_keys("aaa bbb lll\C-[0")
assert_line_around_cursor('', 'aaa bbb lll')
# ed_next_char should move cursor right and cancel vi_next_char
input_keys('f')
input_key_by_symbol(:ed_next_char)
input_keys('l')
assert_line_around_cursor('aa', 'a bbb lll')
# ed_next_char should move cursor right and cancel delete_meta
input_keys('d')
input_key_by_symbol(:ed_next_char)
input_keys('l')
assert_line_around_cursor('aaa ', 'bbb lll')
end

def test_unimplemented_vi_command_should_be_no_op
input_keys("abc\C-[h")
assert_line_around_cursor('a', 'bc')
Expand All @@ -727,12 +772,16 @@ def test_unimplemented_vi_command_should_be_no_op
end

def test_vi_yank
input_keys("foo bar\C-[0")
assert_line_around_cursor('', 'foo bar')
input_keys("foo bar\C-[2h")
assert_line_around_cursor('foo ', 'bar')
input_keys('y3l')
assert_line_around_cursor('', 'foo bar')
assert_line_around_cursor('foo ', 'bar')
input_keys('P')
assert_line_around_cursor('fo', 'ofoo bar')
assert_line_around_cursor('foo ba', 'rbar')
input_keys('3h3yhP')
assert_line_around_cursor('foofo', 'o barbar')
input_keys('yyP')
assert_line_around_cursor('foofofoofoo barba', 'ro barbar')
end

def test_vi_end_word_with_operator
Expand Down