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

Revert #335 (Trap TSTP to handle C-z) #535

Merged
merged 1 commit into from
Apr 24, 2023
Merged

Revert #335 (Trap TSTP to handle C-z) #535

merged 1 commit into from
Apr 24, 2023

Conversation

brasic
Copy link
Contributor

@brasic brasic commented Apr 24, 2023

Fixes #466

#335 was an effort to address #321 (ed_quoted_insert doesn't work properly) but per the reporter the change did not work correctly.

Moreover, it introduced a major regression: Shell job control stopped working in all applications that use reline, notably IRB.

Shells send SIGTSTP in response to SUSP (C-z) to implement the suspension part of job control. Adding a handler for SIGTSTP opts out of this functionality. For a line oriented terminal program this should be avoided when possible (not to mention, this behavior diverges from readline's)

cc @aycabta

This PR was an effort to address ruby#321 (ed_quoted_insert doesn't work
properly) but per the reporter it did not work correctly.

Moreover, it introduced a major regression: Shell job control stopped
working in all applications that use reline, notably IRB.

Bash and other shells send SIGTSTP in response to C-z to implement job
suspension. Handling SIGSTP opts out of this functionality. For a
line oriented terminal program this should be avoided (not to mention,
this behavior diverges from readline's)
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.

Thank you, It make sense to revert it 👍

@tompng tompng merged commit 26383d2 into ruby:master Apr 24, 2023
30 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Apr 24, 2023
(ruby/reline#535)

This PR was an effort to address #321 (ed_quoted_insert doesn't work
properly) but per the reporter it did not work correctly.

Moreover, it introduced a major regression: Shell job control stopped
working in all applications that use reline, notably IRB.

Bash and other shells send SIGTSTP in response to C-z to implement job
suspension. Handling SIGSTP opts out of this functionality. For a
line oriented terminal program this should be avoided (not to mention,
this behavior diverges from readline's)

ruby/reline@26383d25b8

Co-authored-by: Carl Brasic <cbrasic@drwholdings.com>
@tompng
Copy link
Member

tompng commented Apr 24, 2023

(memo for the future)
Reading char like C-z can be implement by STDIN.raw(intr: false){STDIN.getc} instead of using Signal.trap

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.

Suspend not working in IRB Ruby 3.1.2
3 participants