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.

Sign up for GitHub

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

Jump to bottom

Ctrl-C in command substitution exits parent shell #467

Closed
jyn514 opened this issue Aug 18, 2019 · 3 comments
Closed

Ctrl-C in command substitution exits parent shell #467

jyn514 opened this issue Aug 18, 2019 · 3 comments
Labels
high-priority pending-release signal-handling

Comments

@jyn514
Copy link
Contributor

jyn514 commented Aug 18, 2019 •
edited
Loading

Repro steps:

(bash) ~ $ osh --rcfile /dev/null
osh$ `sleep 1`
^C
(bash) ~ $

Expected behavior: osh sends interrupt to command substitution, but continues reading input and behaving as an interactive shell (same as bash and dash)

Actual behavior: osh exits completely!

This looks like it was introduced by 1191238, which enables 'Ctrl-C' during command substitions. osh should distinguish between interrupting the substitution and interrupting the parent shell.
@andychu
Copy link
Contributor

andychu commented Sep 18, 2019 •
edited
Loading

OK I reproduced this... though my memory of how it works is already fuzzy :-/

In any case, thanks for the report and testing!

@andychu
Copy link
Contributor

andychu commented Jan 5, 2022

Hm looks like this fell through the cracks, should take another look at it

andychu pushed a commit that referenced this issue Jan 21, 2022
[interactive] Handle Ctrl-C during command sub.
d958daf 
Add cases to test/interactive.py to verify it

TODO: test that exit codes match bash and other shells.  It looks like
they use exit code 130 consistently.

Addresses issue #467.
@andychu andychu mentioned this issue Jan 22, 2022
Closed
andychu pushed a commit that referenced this issue Jan 26, 2022
[test/interactive] Print a table of results.
2009f8f 
Support selecting tests with --range and so forth.

Mark some tests skipped.  Now it's clear that #467 was incompletely
fixed, since the exit code isn't 130.  Ditto for the wait builtin, which
was an earlier bug.

This is part of #1077.
andychu pushed a commit that referenced this issue Jan 26, 2022
[interactive] Fix exit code after Ctrl-C to be 130.
463e21d 
This finishes the fix for issue #467.

Also fix exit code after parse error -- it is now 2.
@andychu
Copy link
Contributor

andychu commented Jan 30, 2022 •
edited
Loading

Fixed in 0.9.7: https://www.oilshell.org/blog/2022/01/notes-themes.html

Thanks for the report! As mentioned in the blog post, I probably subconsciously put this off because we didn't have a good test harness.

@andychu andychu closed this as completed Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
high-priority pending-release signal-handling
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@andychu @jyn514