[comp_ui] Keep track of screen width in GNU readline.

OSH now queries this state, instead of trying to keep track of it
ourselves.

So OSH behaves like bash and other shells here:

- If trap 'echo X' SIGWINCH, then SIGWINCH will interrupt the wait
  builtin.
- Otherwise it is ignored.

We take care to only enable the SIGWINCH handler inside call_readline(),
when shell code isn't running.

When the TAB completion callback is invoked, we do the reverse: we
disable our SIGWINCH tracking.

So we can lose some resizes, but this is what other shells do.  TODO:
Test this out a bit more.

Addresses #1067.

build/detect-readline.c

@@ -15,5 +15,8 @@ int main(void) {
      This line will break the build otherwise: https://git.io/vhZ3B */
   rl_event_hook = test_event_hook;
 
+  /* TODO: We could also test other functions we use, like rl_resize_terminal()
+   */
+
   return 0;
 }

core/comp_ui.py

@@ -8,7 +8,10 @@
 
 from core import ansi
 from core import completion
+from core.pyerror import log
+
 import libc
+import line_input
 
 from typing import Any, List, Optional, Dict, IO, TYPE_CHECKING
 if TYPE_CHECKING:
@@ -101,6 +104,7 @@ def PrintCandidates(self, *args):
     try:
       self._PrintCandidates(*args)
     except Exception as e:
+      log('Unexpected exception from _PrintCandidates')
       if 0:
         import traceback
         traceback.print_exc()
@@ -296,7 +300,7 @@ def _PrintLong(matches,  # type: List[str]
   return num_lines
 
 
-class NiceDisplay(_IDisplay):
+class ZshLikeDisplay(_IDisplay):
   """Methods to display completion candidates and other messages.
 
   This object has to remember how many lines we last drew, in order to erase
@@ -307,7 +311,6 @@ class NiceDisplay(_IDisplay):
   - displaying descriptions of flags and builtins
   """
   def __init__(self,
-               term_width,  # type: int
                comp_state,  # type: State
                prompt_state,  # type: PromptState
                debug_f,  # type: _DebugFile
@@ -324,7 +327,6 @@ def __init__(self,
     _IDisplay.__init__(self, comp_state, prompt_state, num_lines_cap, f,
                        debug_f)
     self.readline_mod = readline_mod
-    self.term_width = term_width
     self.width_is_dirty = False
 
     self.bold_line = bold_line
@@ -373,7 +375,10 @@ def _PrintCandidates(self, unused_subst, matches, unused_max_match_len):
     # Variables set by the completion generator.  They should always exist,
     # because we can't get "matches" without calling that function.
     display_pos = self.comp_state.display_pos
-    self.debug_f.log('DISPLAY POS in _PrintCandidates = %d', display_pos)
+
+    self.debug_f.log(
+        'ZshLikeDisplay::_PrintCandidates term_width = %d, display_pos = %d',
+        term_width, display_pos)
 
     self.f.write('\n')
 
@@ -409,7 +414,7 @@ def _PrintCandidates(self, unused_subst, matches, unused_max_match_len):
     # Calculate max length after stripping prefix.
     max_match_len = max(len(m) for m in to_display)
 
-    # TODO: NiceDisplay should truncate when max_match_len > term_width?
+    # TODO: ZshLikeDisplay should truncate when max_match_len > term_width?
     # Also truncate when a single candidate is super long?
 
     # Print and go back up.  But we have to ERASE these before hitting enter!
@@ -504,17 +509,10 @@ def EraseLines(self):
     self.f.flush()  # Without this, output will look messed up
 
   def _GetTerminalWidth(self):
-    # type: () -> int
-    if self.width_is_dirty:
-      try:
-        self.term_width = libc.get_terminal_width()
-      except IOError:
-        # This shouldn't raise IOError because we did it at startup!  Under
-        # rare circumstances stdin can change, e.g. if you do exec <&
-        # input.txt.  So we have a fallback.
-        self.term_width = 80
-      self.width_is_dirty = False
-    return self.term_width
+    # This function doesn't make syscalls; it queries globals inside GNU
+    # readline
+    _, cols = line_input.get_screen_size()
+    return cols
 
   def OnWindowChange(self):
     # type: () -> None

core/completion.py

@@ -504,7 +504,7 @@ def Matches(self, comp):
     if val.tag_() != value_e.MaybeStrArray:
       log('ERROR: COMPREPLY should be an array, got %s', val)
       return []
-    self.log('COMPREPLY %s', val)
+    #self.log('COMPREPLY %s', val)
 
     # Return this all at once so we don't have a generator.  COMPREPLY happens
     # all at once anyway.

core/pyos.py

@@ -280,12 +280,9 @@ def InitInteractiveShell(self, display):
     # This prevents Ctrl-Z from suspending OSH in interactive mode.
     signal.signal(signal.SIGTSTP, signal.SIG_IGN)
 
-    # 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())
-
-    # Disabled because it causes wait builtin to abort (issue 1067)
-    signal.signal(signal.SIGWINCH, signal.SIG_IGN)
+    # NOTE: native/line_input.c registers SIGWINCH *sometimes*
+    # It takes care not to burden the shell with it, e.g. to avoid issue 1067
+    # where the 'wait' builtin is interrupted on SIGWINCH
 
   def AddUserTrap(self, sig_num, handler):
     # type: (int, Any) -> None

core/shell.py

@@ -609,16 +609,13 @@ def Main(lang, arg_r, environ, login_shell, loader, line_input):
       root_comp = completion.RootCompleter(ev, mem, comp_lookup, compopt_state,
                                            comp_ui_state, comp_ctx, debug_f)
 
-      term_width = 0
-      if flag.completion_display == 'nice':
-        try:
-          term_width = libc.get_terminal_width()
-        except IOError:  # stdin not a terminal
-          pass
-
-      if term_width != 0:
-        display = comp_ui.NiceDisplay(term_width, comp_ui_state, prompt_state,
-                                      debug_f, line_input)  # type: comp_ui._IDisplay
+      if flag.completion_display == 'z_readline':
+        display = comp_ui.ZshLikeDisplay(comp_ui_state, prompt_state,
+                                         debug_f, line_input)  # type: comp_ui._IDisplay
+
+      elif flag.completion_display == 'b_readline':
+        raise error.Usage('b_readline style not implemented')
+
       else:
         display = comp_ui.MinimalDisplay(comp_ui_state, prompt_state, debug_f)
 

frontend/flag_def.py

@@ -217,7 +217,9 @@ def _AddShellOptions(spec):
     default='abbrev-text')
 
 # Defines completion style.
-OSH_SPEC.LongFlag('--completion-display', ['minimal', 'nice'], default='nice')
+OSH_SPEC.LongFlag('--completion-display',
+    ['minimal', 'b_readline', 'z_readline'], default='z_readline')
+
 # TODO: Add option for Oil prompt style?  RHS prompt?
 
 # Don't reparse a[x+1] and ``.  Only valid in -n mode.

native/libc.c

@@ -383,6 +383,7 @@ static PyMethodDef methods[] = {
   {"gethostname", socket_gethostname, METH_NOARGS, ""},
 
   // ioctl() to get the terminal width.
+  // NOTE: NO LONGER USED since we get let GNU readline keep track of it.
   {"get_terminal_width", func_get_terminal_width, METH_NOARGS, ""},
 
   // Get the display width of a string. Throw an exception if the string is invalid UTF8.

native/line_input.c

@@ -39,9 +39,6 @@
 /* Define if you have readline 4.0 */
 #define HAVE_RL_PRE_INPUT_HOOK 1
 
-/* Define if you have readline 4.0 */
-#define HAVE_RL_RESIZE_TERMINAL 1
-
 /* ------------------------------------------------------------------------- */
 
 #if defined(HAVE_SETLOCALE)
@@ -673,6 +670,17 @@ py_resize_terminal(PyObject *self, PyObject *noarg)
     Py_RETURN_NONE;
 }
 
+/* Added for OSH.  We need to call this in our SIGWINCH handler so global
+ * variables in readline get updated. */
+static PyObject *
+py_get_screen_size(PyObject *self, PyObject *noarg)
+{
+    int rows = -1;  // a hint that it failed
+    int cols = -1;
+    rl_get_screen_size(&rows, &cols);
+    return Py_BuildValue("(i,i)", rows, cols);
+}
+
 /* Exported function to insert text into the line buffer */
 
 static PyObject *
@@ -754,7 +762,15 @@ static struct PyMethodDef readline_methods[] = {
 #ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER
     {"clear_history", py_clear_history, METH_NOARGS, doc_clear_history},
 #endif
+
+    // TODO: why aren't these in
+    // build/oil-defs/native/line_input.c/readline_methods.def?
+
+    // OVM_MAIN.  No longer need this?
     {"resize_terminal", py_resize_terminal, METH_NOARGS, ""},
+    // OVM_MAIN.  Added this so we don't handle SIGWINCH
+    {"get_screen_size", py_get_screen_size, METH_NOARGS, ""},
+
     {0, 0}
 };
 #endif
@@ -817,6 +833,9 @@ on_pre_input_hook()
 }
 #endif
 
+static volatile sig_atomic_t sigwinch_received;
+// User handler that we temporarily replace, e.g. for trap 'echo X' SIGWINCH
+static PyOS_sighandler_t sigwinch_ohandler;
 
 /* C function to call the Python completion_display_matches */
 
@@ -841,9 +860,16 @@ on_completion_display_matches_hook(char **matches,
             goto error;
     }
 
+    // Do the reverse of what happens in in call_readline.  When we run shell
+    // code, we may want SIGWINCH to be SIG_IGN.
+    PyOS_sighandler_t temp = PyOS_setsig(SIGWINCH, sigwinch_ohandler);
+
     r = PyObject_CallFunction(completion_display_matches_hook,
                               "sOi", matches[0], m, max_length);
 
+    PyOS_sighandler_t restored = PyOS_setsig(SIGWINCH, temp);
+    assert(restored == sigwinch_ohandler);
+
     Py_DECREF(m); m=NULL;
 
     if (r == NULL ||
@@ -865,19 +891,18 @@ on_completion_display_matches_hook(char **matches,
 
 #endif
 
-#ifdef HAVE_RL_RESIZE_TERMINAL
-static volatile sig_atomic_t sigwinch_received;
-static PyOS_sighandler_t sigwinch_ohandler;
-
 static void
 readline_sigwinch_handler(int signum)
 {
+    // set flag for input select() loop to check
     sigwinch_received = 1;
+
+    // call the original handler
     if (sigwinch_ohandler &&
-            sigwinch_ohandler != SIG_IGN && sigwinch_ohandler != SIG_DFL)
+        sigwinch_ohandler != SIG_IGN && sigwinch_ohandler != SIG_DFL) {
         sigwinch_ohandler(signum);
+    }
 }
-#endif
 
 /* C function to call the Python completer. */
 
@@ -980,10 +1005,6 @@ setup_readline(void)
     /* Bind both ESC-TAB and ESC-ESC to the completion function */
     rl_bind_key_in_map ('\t', rl_complete, emacs_meta_keymap);
     rl_bind_key_in_map ('\033', rl_complete, emacs_meta_keymap);
-#ifdef HAVE_RL_RESIZE_TERMINAL
-    /* Set up signal handler for window resize */
-    sigwinch_ohandler = PyOS_setsig(SIGWINCH, readline_sigwinch_handler);
-#endif
     /* Set our hook functions */
     rl_startup_hook = on_startup_hook;
 #ifdef HAVE_RL_PRE_INPUT_HOOK
@@ -1055,11 +1076,6 @@ readline_until_enter_or_signal(char *prompt, int *signal)
 #ifdef HAVE_RL_CATCH_SIGNAL
     rl_catch_signals = 0;
 #endif
-    /* OVM_MAIN: Oil is handling SIGWINCH, so readline shouldn't handle it.
-     * Without this line, strace reveals that GNU readline is constantly
-     * turning it on and off.
-     * */
-    rl_catch_sigwinch = 0;
 
     rl_callback_handler_install (prompt, rlhandler);
     FD_ZERO(&selectset);
@@ -1077,13 +1093,11 @@ readline_until_enter_or_signal(char *prompt, int *signal)
             struct timeval *timeoutp = NULL;
             if (PyOS_InputHook)
                 timeoutp = &timeout;
-#ifdef HAVE_RL_RESIZE_TERMINAL
             /* Update readline's view of the window size after SIGWINCH */
             if (sigwinch_received) {
                 sigwinch_received = 0;
                 rl_resize_terminal();
             }
-#endif
             FD_SET(fileno(rl_instream), &selectset);
             /* select resets selectset if no input was available */
             has_input = select(fileno(rl_instream) + 1, &selectset,
@@ -1183,8 +1197,16 @@ call_readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
 #endif
     }
 
+    /* OVM_MAIN: This handle is active while we're NOT executing shell code
+     * It sets a flag for the select() loop to check
+     */
+    sigwinch_ohandler = PyOS_setsig(SIGWINCH, readline_sigwinch_handler);
+
     p = readline_until_enter_or_signal(prompt, &signal);
 
+    PyOS_sighandler_t restored = PyOS_setsig(SIGWINCH, sigwinch_ohandler);
+    assert(restored == readline_sigwinch_handler);
+
     /* we got an interrupt signal */
     if (signal) {
         RESTORE_LOCALE(saved_locale)

native/line_input_test.py

@@ -22,6 +22,10 @@ class LineInputTest(unittest.TestCase):
   def testMatchOshToken(self):
     print(dir(line_input))
 
+  def testGetScreenSize(self):
+    screen_size = line_input.get_screen_size()
+    print('screen size %s' % (screen_size,))
+
 
 if __name__ == '__main__':
   unittest.main()