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

Allow composing user-defined WINCH signal traps #736

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
18 changes: 13 additions & 5 deletions lib/reline/line_editor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,14 @@ def handle_signal
@rendered_screen.lines = []
@rendered_screen.cursor_y = 0
render_differential
case @old_winch_trap
when 'DEFAULT', 'SYSTEM_DEFAULT', 'IGNORE'
# Do nothing
when 'EXIT'
exit
else
@old_winch_trap.call if @old_winch_trap.respond_to?(:call)
end
Comment on lines +182 to +189
Copy link
Member

Choose a reason for hiding this comment

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

I think @old_winch_trap.call if @old_winch_trap.respond_to?(:call) is enough.

I don't think Signal.trap('WINCH', 'EXIT') will be used. Even if it is set, we can think that Reline override and ignore it.
We can consider adding when 'EXIT'; exit when there is a request with a use case.

end

private def handle_interrupted
Expand All @@ -191,29 +199,29 @@ def handle_signal
Reline::IOGate.move_cursor_column 0
@rendered_screen.lines = []
@rendered_screen.cursor_y = 0
case @old_trap
case @old_int_trap
when 'DEFAULT', 'SYSTEM_DEFAULT'
raise Interrupt
when 'IGNORE'
# Do nothing
when 'EXIT'
exit
else
@old_trap.call if @old_trap.respond_to?(:call)
@old_int_trap.call if @old_int_trap.respond_to?(:call)
end
end

def set_signal_handlers
Reline::IOGate.set_winch_handler do
@old_winch_trap = Reline::IOGate.set_winch_handler do
Copy link
Member

Choose a reason for hiding this comment

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

Reline::IOGate.set_winch_handler does not return old_winch_trap.

Reline::ANSI returns SIGCONT trap
Reline::Windows returns new_winch_trap
Reline::Dumb returns nil

I think we should do old_winch_trap.call inside lib/reline/io/ansi.rb.

def set_winch_handler(&handler)
  @old_winch_handler = Signal.trap('WINCH') do
    # do it here?
  end
  @old_cont_handler = Signal.trap('CONT') do
    # maybe we can fix the same problem with SIGCONT here
  end
end

Copy link
Author

Choose a reason for hiding this comment

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

Ah, great point. I've only been using the ANSI adapter, so didn't think to check the others and what they return. Will think on how to shift this down one level

@resized = true
end
@old_trap = Signal.trap('INT') do
@old_int_trap = Signal.trap('INT') do
@interrupted = true
end
end

def finalize
Signal.trap('INT', @old_trap)
Signal.trap('INT', @old_int_trap)
end

def eof?
Expand Down
Loading