Refactor waiting_proc and waiting_operator_proc #649
Conversation
tompng
force-pushed
the
waiting_proc_fix
branch
from
3d40721 to
59c63c9
Compare
test/reline/test_key_actor_vi.rb
Outdated
| input_keys('d2h') | ||
| assert_line_around_cursor('aaa bb', 'c') | ||
| input_keys('2d3h') | ||
| assert_line_around_cursor('aaa', 'c') |
There was a problem hiding this comment.
When I perform the same action in Vim, I get a different result. Is this intentional?
- Enter
aaa bbc - Place the cursor on
c - Enter
2d3h - Only
cremains
There was a problem hiding this comment.
Nice catch, looks like I misunderstood the correct behavior.
Updated the test to match with readline, fix implementation to use 2*3 = 6 as vi_arg in that case
vi_arg and vi_waiting_operator_arg should be multiplied
test/reline/test_key_actor_vi.rb
Outdated
| # p should not paste yanked text because yank_meta is canceled | ||
| input_keys('dchdyhcdhcyhydhychyyhp') | ||
| assert_line_around_cursor('a', 'aa bbb ccc') | ||
| input_keys('dchdyhcdhcyhydhPychP') |
There was a problem hiding this comment.
Can you split the input? This input is a bit complex. 😵💫
| current_waiting_operator_proc.(byte_pointer_diff) | ||
| @waiting_operator_proc = old_waiting_operator_proc | ||
| } | ||
| send(@vi_waiting_operator, byte_pointer_diff) |
There was a problem hiding this comment.
For my learning: Why pass byte_pointer_diff? Are multi-byte characters allowed?
There was a problem hiding this comment.
Yes. multi-byte characters are allowed.
vi_waiting_operator will do
- save cursor position (represented by
@byte_pointer) - accept VI_MOTIONS. cursor position moves. (
@line_indexdoes not change) - 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.
There was a problem hiding this comment.
I understood. Thanks for explaining!
| if @vi_arg | ||
| @vi_arg = nil | ||
| end | ||
| @vi_arg = nil |
There was a problem hiding this comment.
For my learning: So all we need to do here is initialize vi_arg, why don't we call cleanup_waiting?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
I misunderstood the relationship between @waiting_operator and @vi_waiting_operator. Thanks!
(ruby/reline#649) * Fix waiting_proc precedence * Fix waiting_operator bugs * Add waiting_proc and vi_waiting_operator test * Fix vi waiting operator arg number vi_arg and vi_waiting_operator_arg should be multiplied * Implement `yy` copies whole line in vi_command mode * Simplify incremental search cancel test * Add complex vi test with waiting_proc and vi_waiting_operator, split test input ruby/reline@777dffae1c
* master: (29 commits) Not all `nm`s support the `--help` option Emit a performance warning when redefining specially optimized methods YJIT: A64: Avoid intermediate register in `opt_and` and friends (ruby#10509) [ruby/reline] Remove not implemented method (ruby/reline#680) [ruby/reline] Fix vi_to_column which was broken (ruby/reline#679) Include more debug information in test_uplus_minus [Universal parser] DeVALUE of p->debug_lines and ast->body.script_lines Add more assertions in `test_uplus_minus` `super{}` doesn't use block fix incorrect warning. Update default gems list at fc36394 [ci skip] [ruby/optparse] bump up to 0.5.0 show warning for unused block Emit `warn` event for duplicated hash keys on ripper [ruby/reline] Refactored Default Key Bindings (ruby/reline#678) [ruby/reline] Refactor waiting_proc and waiting_operator_proc (ruby/reline#649) [pty] Fix missing `or` [pty] Fix `ptsname_r` fallback [ruby/irb] Allow defining custom commands in IRB (ruby/irb#886) [ruby/reline] Support `menu-complete-backward` command for upward navigation (ruby/reline#677) ...
2 participants
Test will conflict with #645
Fixes #605
waiting_proc
Most waiting_proc will be cancelled when ARROW keys (and other symbol key) is pressed, but Reline did not implement cancel
Fixes except ed_quoted_insert.
waiting_operator_proc
Vi mode's waiting_operator_proc was complex.
Reline's problem with waiting_operator
df_does not terminate delete_metaddshould clear lineccshould clear linedcdycdcyydycyyshould cancel waiting_operator3d5h, 5 should be used as@vi_argbut Reline does notFixes all of them.