Skip to content

Use em_delete in key_delete #504

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

Merged
tompng merged 3 commits into from
Mar 24, 2023
Merged

Use em_delete in key_delete #504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e97956
Test existing behavior
etiennebarrie Mar 24, 2023
83588f4
Check if line empty first in em_delete
etiennebarrie Jan 17, 2023
c0d4086
Use em_delete in key_delete
etiennebarrie Dec 22, 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
6 changes: 4 additions & 2 deletions lib/reline/line_editor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1944,8 +1944,10 @@ def finish
end

private def key_delete(key)
if @config.editing_mode_is?(:vi_insert, :emacs)
if @config.editing_mode_is?(:vi_insert)
ed_delete_next_char(key)
elsif @config.editing_mode_is?(:emacs)
em_delete(key)
Copy link
Member

@tompng tompng Mar 23, 2023

Choose a reason for hiding this comment

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

Could you add a test for this?
I think this part is trying to test key_delete but missing the scenario you reported

reline/test/reline/test_key_actor_emacs.rb

Lines 452 to 460 in 3aeea2a

def test_ed_delete_next_char
input_keys('abc')
assert_cursor(3)
assert_cursor_max(3)
@line_editor.input_key(Reline::Key.new(:key_delete, :key_delete, false))
assert_cursor(3)
assert_cursor_max(3)
assert_line('abc')
end

Copy link
Contributor Author

@etiennebarrie etiennebarrie Mar 24, 2023

Choose a reason for hiding this comment

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

Thank you for pointing the tests. I've added a test in the third commit (making this change), that checks that the cursor is preserved. I also renamed this test in the first commit to clarify it's testing key_delete.

end
end

Expand Down Expand Up @@ -2651,7 +2653,7 @@ def finish
alias_method :kill_whole_line, :em_kill_line

private def em_delete(key)
if (not @is_multiline and @line.empty?) or (@is_multiline and @line.empty? and @buffer_of_lines.size == 1)
if @line.empty? and (not @is_multiline or @buffer_of_lines.size == 1) and key == "\C-d".ord
@line = nil
if @buffer_of_lines.size > 1
scroll_down(@highest_in_all - @first_line_started_from)
Expand Down
27 changes: 26 additions & 1 deletion test/reline/test_key_actor_emacs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,12 @@ def test_em_delete_for_mbchar_by_plural_code_points
assert_line("き\u3099")
end

def test_em_delete_ends_editing
input_keys("\C-d") # quit from inputing
assert_line(nil)
assert(@line_editor.finished?)
end

def test_ed_clear_screen
refute(@line_editor.instance_variable_get(:@cleared))
input_keys("\C-l", false)
Expand All @@ -449,7 +455,7 @@ def test_ed_clear_screen_with_inputed
assert_line('abc')
end

def test_ed_delete_next_char
def test_key_delete
input_keys('abc')
assert_cursor(3)
assert_cursor_max(3)
Expand All @@ -459,6 +465,25 @@ def test_ed_delete_next_char
assert_line('abc')
end

def test_key_delete_does_not_end_editing
@line_editor.input_key(Reline::Key.new(:key_delete, :key_delete, false))
assert_cursor(0)
assert_cursor_max(0)
assert_line('')
refute(@line_editor.finished?)
end

def test_key_delete_preserves_cursor
input_keys('abc')
input_keys("\C-b", false)
assert_cursor(2)
assert_cursor_max(3)
@line_editor.input_key(Reline::Key.new(:key_delete, :key_delete, false))
assert_cursor(2)
assert_cursor_max(2)
assert_line('ab')
end

def test_em_next_word
assert_byte_pointer_size('')
assert_cursor(0)
Expand Down