-
Notifications
You must be signed in to change notification settings - Fork 83
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
base: master
Are you sure you want to change the base?
Conversation
Resolves ruby#735 Just as the `handle_interrupted` method runs Reline's interrupt hook logic and then runs any user-defined interrupt hook logic, `handle_resized` should run Reline's resize hook logic and then run any user-defined resize logic. This allows user CLI's with additional content in addition to the Reline prompt to ensure that additional content is rendered properly as the terminal is resized. Note: This is my best first attempt at this solution as someone who has only read the source code here today. There may be details/considerations that are important that I missed or didn't consider. I am very open to changes as necessary. cc: @st0012
end | ||
end | ||
|
||
def set_signal_handlers | ||
Reline::IOGate.set_winch_handler do | ||
@old_winch_trap = Reline::IOGate.set_winch_handler do |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
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 |
There was a problem hiding this comment.
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.
Resolves #735
Just as the
handle_interrupted
method runs Reline's interrupt hook logic and then runs any user-defined interrupt hook logic,handle_resized
should run Reline's resize hook logic and then run any user-defined resize logic.This allows user CLI's with additional content in addition to the Reline prompt to ensure that additional content is rendered properly as the terminal is resized.
Note: This is my best first attempt at this solution as someone who has only read the source code here today. There may be details/considerations that are important that I missed or didn't consider. I am very open to changes as necessary.
cc: @st0012