Skip to content

Commit

Permalink
[interactive] Fix bug where Ctrl-C didn't abort entire line.
Browse files Browse the repository at this point in the history
The top-level 'except KeyboardInterrupt' I just added handles it
correctly.  We don't need it around builtins and assignment builtins.

spec/stateful:
- Rename files for consistency
- signals: Compare against dash and zsh.  The behavior is quite
  consistent!

This is progress toward #1079, since we're getting rid of some
KeyboardInterrupt.
  • Loading branch information
Andy C committed Feb 3, 2022
1 parent a17360d commit b1e1823
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 15 deletions.
7 changes: 0 additions & 7 deletions core/executor.py
Expand Up @@ -138,13 +138,6 @@ def RunBuiltin(self, builtin_id, cmd_val):
# e.g. 'type' doesn't accept flag '-x'
self.errfmt.PrefixPrint(e.msg, prefix='%r ' % arg0, span_id=e.span_id)
status = 2 # consistent error code for usage error
except KeyboardInterrupt:
if self.exec_opts.interactive():
print('') # newline after ^C
status = 130 # 128 + 2 for SIGINT
else:
# Abort a batch script
raise
finally:
# Flush stdout after running ANY builtin. This is very important!
# Silence errors like we did from 'echo'.
Expand Down
6 changes: 0 additions & 6 deletions osh/cmd_eval.py
Expand Up @@ -289,12 +289,6 @@ def _RunAssignBuiltin(self, cmd_val):
e.span_id = self.errfmt.CurrentLocation()
self.errfmt.PrefixPrint(e.msg, prefix='%r ' % arg0, span_id=e.span_id)
status = 2 # consistent error code for usage error
except KeyboardInterrupt:
if self.exec_opts.interactive():
print('') # newline after ^C
status = 130 # 128 + 2 for SIGINT
else:
raise
finally:
try:
sys.stdout.flush()
Expand Down
File renamed without changes.
File renamed without changes.
30 changes: 28 additions & 2 deletions spec/stateful/signals.py
Expand Up @@ -143,7 +143,8 @@ def sigwinch_untrapped_wait(sh):
sh.expect('status=0')


@register()
# dash and mksh don't have pipestatus
@register(skip_shells=['dash', 'mksh'])
def sigwinch_untrapped_wait_n(sh):
'untrapped SIGWINCH during wait -n'

Expand Down Expand Up @@ -178,7 +179,8 @@ def sigwinch_untrapped_external(sh):
sh.expect('status=0')


@register()
# dash doesn't have pipestatus
@register(skip_shells=['dash'])
def sigwinch_untrapped_pipeline(sh):
'untrapped SIGWINCH during pipeline'

Expand Down Expand Up @@ -224,6 +226,12 @@ def t4(sh):
sh.expect('status=130')


# TODO:
# - Expand this to call kinds of reads
# - note that mksh only has some of them
# - Expand to trapped and untrapped
# - Expand to Ctrl-C vs. SIGWINCH

@register()
def t2(sh):
'Ctrl-C during read builtin'
Expand Down Expand Up @@ -275,6 +283,24 @@ def c_wait_n(sh):
sh.expect('status=130')


@register()
def c_wait_line(sh):
'Ctrl-C (untrapped) cancels entire interactive line'

# the echo should be cancelled
sh.sendline('sleep 10 & wait; echo status=$?')

time.sleep(0.1)

# TODO: actually send Ctrl-C through the terminal, not SIGINT?
sh.sendintr() # SIGINT

sh.expect(r'.*\$') # expect prompt

sh.sendline('echo done=$?')
sh.expect('done=130')


@register()
def t5(sh):
'Ctrl-C during Command Sub (issue 467)'
Expand Down
5 changes: 5 additions & 0 deletions test/stateful.sh
Expand Up @@ -29,6 +29,11 @@ readonly BASE_DIR=_tmp/stateful
signals() {
spec/stateful/signals.py \
$OSH bash "$@"

# They now pass for dash and mksh, with wait -n and PIPESTATUS skipped.
# zsh doesn't work now, but could if the prompt was changed to $ ?

#$OSH bash dash mksh "$@"
}

interactive() {
Expand Down

0 comments on commit b1e1823

Please sign in to comment.