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

Fix [nil] is passed to auto_indent_proc when exit with CTRL+d #571

Merged
merged 2 commits into from
Jul 18, 2023

Conversation

osyo-manga
Copy link
Contributor

Fix it #556
auto_indent_proc will argument ["] instead of [nil].

@tompng
Copy link
Member

tompng commented Jul 16, 2023

It seems [nil] is passed to auto_indent_proc only when Ctrl+d is pressed with empty input. In that case, I think reline doesn't need to call auto_indent_proc at all.

Could you investigate why auto_indent_proc is called and can the call be skipped? If it's hard for us to fix auto_indent_proc call, I think this fix is good and can be merged.

@osyo-manga
Copy link
Contributor Author

@tompng hi.

Could you investigate why auto_indent_proc is called and can the call be skipped? If it's hard for us to fix auto_indent_proc call, I think this fix is good and can be merged.

If C-d is entered, #finish is called here to finish the process.

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)
end
Reline::IOGate.move_cursor_column(0)
@eof = true
finish

So the simple modification is as follows.

diff --git a/lib/reline/line_editor.rb b/lib/reline/line_editor.rb
index 7ad56a3..5b98df0 100644
--- a/lib/reline/line_editor.rb
+++ b/lib/reline/line_editor.rb
@@ -1605,7 +1605,7 @@ class Reline::LineEditor
     else
       @just_cursor_moving = false
     end
-    if @is_multiline and @auto_indent_proc and not simplified_rendering?
+    if @is_multiline and @auto_indent_proc and not simplified_rendering? and not finished?
       process_auto_indent
     end
   end

However, #finished? returns true even when exiting outside of C-d.
So there is a possibility that auto_indent_proc will not be called even if the program terminates outside of C-d.

@tompng
Copy link
Member

tompng commented Jul 17, 2023

When auto_indent_proc returns non-zero value

require 'reline'
Reline.auto_indent_proc=->(*){2}
Reline.readmultiline('>'){true}

process_auto_indent fails in this line.

@line = ' ' * new_indent + @line.lstrip

We need to find a way to skip calling process_auto_indent.

Thanks for investigating and description about finished?. Yes, we need to recalculate indent in normal finished case.
How about adding and @line instead of and not finished?

osyo-manga added a commit to osyo-manga/reline that referenced this pull request Jul 17, 2023
@osyo-manga
Copy link
Contributor Author

@tompng OK, Fixed it. Thanks.
osyo-manga@b203474

@st0012 st0012 added the bug Something isn't working label Jul 18, 2023
Copy link
Member

@tompng tompng left a comment

Choose a reason for hiding this comment

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

👍

@tompng tompng merged commit 0924f2a into ruby:master Jul 18, 2023
matzbot pushed a commit to ruby/ruby that referenced this pull request Jul 18, 2023
CTRL+d
(ruby/reline#571)

* Fix [nil] is passed to auto_indent_proc when exit with CTRL+d

Fix it ruby/reline#556

* not call auto_indent_proc when Ctrl+d.

see: ruby/reline#571 (comment)

ruby/reline@0924f2a075
@osyo-manga osyo-manga deleted the fix-556 branch July 19, 2023 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

3 participants