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

Conversation

fractaledmind
Copy link

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

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
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

Comment on lines +182 to +189
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
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Can't compose user terminal resize logic with Reline's terminal resize logic
2 participants