Skip to content

Commit

Permalink
[interactive] Ignore SIGWINCH in the shell.
Browse files Browse the repository at this point in the history
The line editor is the only thing that needs to know the width, so I
think we should let it handle the problem.

Unfortunately GNU readline is not what we want, but that's a separate
issue.

This means the core/comp_ui.py now doesn't adjust when the window is
resized.  But that dosn't make the shell unusuable.  We should revisit
it later.

Apparently fish and zsh do something different anyway -- they REDRAW on
SIGWINCH.  They don't merely update the width.

- Added a test in test/interactive.py.
- Addresses #1067
  • Loading branch information
Andy C committed Jan 22, 2022
1 parent d958daf commit 3ab4e69
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 9 deletions.
5 changes: 5 additions & 0 deletions core/comp_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,11 @@ def _GetTerminalWidth(self):

def OnWindowChange(self):
# type: () -> None
"""
Note: SIGWINCH handler in core/pyos.py no longer calls this.
The shell process should probably handle SIGWINCH only when blocked in
readline().
"""
# Only do it for the NEXT completion. The signal handler can be run in
# between arbitrary bytecodes, and we don't want a single completion
# display to be shown with different widths.
Expand Down
5 changes: 4 additions & 1 deletion core/pyos.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,10 @@ def InitInteractiveShell(self, display):

# Register a callback to receive terminal width changes.
# NOTE: In line_input.c, we turned off rl_catch_sigwinch.
signal.signal(signal.SIGWINCH, lambda x, y: display.OnWindowChange())
#signal.signal(signal.SIGWINCH, lambda x, y: display.OnWindowChange())

# Disabled because it causes wait builtin to abort (issue 1067)
signal.signal(signal.SIGWINCH, signal.SIG_IGN)

def AddUserTrap(self, sig_num, handler):
# type: (int, Any) -> None
Expand Down
32 changes: 24 additions & 8 deletions test/interactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,22 @@ def get_pid_by_name(name):
# XXX: make sure this is restricted to subprocesses under us.
# This could be problematic on the continuous build if many tests are running
# in parallel.
output = pexpect.run('pgrep --exact --newest cat')
output = pexpect.run('pgrep --exact --newest %s' % name)
return int(output.split()[-1])


def send_signal(name, sig_num):
"""Kill the most recent process matching `name`."""
os.kill(get_pid_by_name(name), sig_num)


# XXX: osh.sendcontrol("z") does not suspend the foreground process :(
#
# why does osh.sendcontrol("c") generate SIGINT, while osh.sendcontrol("z")
# appears to do nothing?
def stop_process__hack(name):
"""Send sigstop to the most recent process matching `name`"""
os.kill(get_pid_by_name(name), signal.SIGSTOP)


def kill_process(name):
"""Kill the most recent process matching `name`."""
os.kill(get_pid_by_name(name), signal.SIGINT)
send_signal(name, signal.SIGSTOP)


class InteractiveTest(object):
Expand Down Expand Up @@ -119,6 +119,22 @@ def __exit__(self, t, v, tb):


def main(argv):
with InteractiveTest('wait builtin then SIGWINCH (issue 1067)') as osh:
osh.sendline('sleep 1 &')
osh.sendline('wait')

time.sleep(0.1)

# simulate window size change
osh.kill(signal.SIGWINCH)

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

osh.sendline('echo status=$?')
osh.expect('status=0')

#return

with InteractiveTest('Ctrl-C during external command') as osh:
osh.sendline('sleep 5')

Expand Down Expand Up @@ -201,7 +217,7 @@ def main(argv):
osh.expect(r".*\$")
osh.sendline("fg")
osh.expect(r"Continue PID \d+")
kill_process("cat")
send_signal("cat", signal.SIGINT)
osh.expect(r".*\$")
osh.sendline("fg")
osh.expect("No job to put in the foreground")
Expand Down

0 comments on commit 3ab4e69

Please sign in to comment.