Cleanup of old completion code.

- We distinguish between progress messages and debug messages:
  - progress messages to a single ui.StatusLine()
  - debug logs go to --debug-file
- crashdump:
  - flatten the JSON for cells
  - Another test case with 'source', which revealed some bugs.

All unit tests pass.

bin/oil.py

@@ -146,9 +146,6 @@ def OshMain(argv0, argv, login_shell):
   pool = alloc.Pool()
   arena = pool.NewArena()
 
-  # TODO: Maybe wrap this initialization sequence up in an oil_State, like
-  # lua_State.
-  status_lines = ui.MakeStatusLines()
   mem = state.Mem(dollar0, argv[arg_r.i + 1:], os.environ, arena)
   funcs = {}
 
@@ -162,8 +159,10 @@ def OshMain(argv0, argv, login_shell):
   parse_ctx = parse_lib.ParseContext(arena, aliases)
 
   if opts.debug_file:
-    util.DEBUG_FILE = fd_state.Open(opts.debug_file, mode='w')
-    util.Debug('Debug file is %s', util.DEBUG_FILE)
+    debug_f = util.DebugFile(fd_state.Open(opts.debug_file, mode='w'))
+  else:
+    debug_f = util.NullDebugFile()
+  debug_f.log('Debug file is %s', opts.debug_file)
 
   # Controlled by env variable, flag, or hook?
   dumper = dev.CrashDumper(os.getenv('OSH_CRASH_DUMP_DIR', ''))
@@ -223,11 +222,11 @@ def OshMain(argv0, argv, login_shell):
     # NOTE: We're using a different evaluator here.  The completion system can
     # also run functions... it gets the Executor through Executor._Complete.
     if HAVE_READLINE:
+      progress_f = ui.StatusLine()
       splitter = legacy.SplitContext(mem)
       ev = word_eval.CompletionWordEvaluator(mem, exec_opts, splitter)
-      status_out = completion.StatusOutput(status_lines, exec_opts)
       completion.Init(readline, pool, builtin.BUILTIN_DEF, mem, funcs,
-                      comp_lookup, status_out, ev, parse_ctx)
+                      comp_lookup, progress_f, debug_f, ev, parse_ctx)
 
     return main_loop.Interactive(opts, ex, c_parser, arena)
 

core/cmd_exec.py

@@ -1164,7 +1164,11 @@ def ExecuteAndCatch(self, node, fork_external=True):
         status = e.StatusCode()
       else:
         raise  # Invalid
+    except util.ParseError as e:
+      self.dumper.MaybeCollect(self)  # Do this before unwinding stack
+      raise
     except util.FatalRuntimeError as e:
+      self.dumper.MaybeCollect(self)  # Do this before unwinding stack
       ui.PrettyPrintError(e, self.arena)
       is_fatal = True
       status = e.exit_status if e.exit_status is not None else 1

core/completion.py

@@ -442,7 +442,7 @@ def _FindLastSimpleCommand(node):
 # VarCompleter.Complete(prefix) -> iter
 # HashKey.Complete(hash_name, prefix) -> iter
 
-def _GetCompletionType(w_parser, c_parser, ev, status_out):
+def _GetCompletionType(w_parser, c_parser, ev, debug_f):
   """
   Parser returns completion state.
   Then we translate that into completion_state_e.
@@ -520,15 +520,14 @@ def _GetCompletionType(w_parser, c_parser, ev, status_out):
     pass
 
   # TODO: Need to show buf... Need a multiline display for debugging?
-  status_out.Write(1,
-      'prev_token %s  cur_token %s  cur_word %s',
+  debug_f.log('prev_token %s  cur_token %s  cur_word %s',
       prev_token, cur_token, cur_word)
-  status_out.Write(2, 'comp_state %s  error %s', comp_state, c_parser.Error())
+  debug_f.log('comp_state %s  error %s', comp_state, c_parser.Error())
   # This one can be multiple lines
-  status_out.Write(3, 'node: %s %s', repr(node) if node else '<Parse Error>',
-                    node.tag if node else '')
+  debug_f.log('node: %s %s', repr(node) if node else '<Parse Error>',
+                node.tag if node else '')
   # This one can be multiple lines
-  status_out.Write(6, 'com_node: %s', repr(com_node) if com_node else '<None>')
+  debug_f.log('com_node: %s', repr(com_node) if com_node else '<None>')
 
   # IMPORTANT: if the last token is Id.Ignored_Space, then we want to add a
   # dummy word!  empty word
@@ -600,21 +599,24 @@ class RootCompleter(object):
   """
   Provide completion of a buffer according to the configured rules.
   """
-  def __init__(self, pool, ev, comp_lookup, var_comp, parse_ctx):
+  def __init__(self, pool, ev, comp_lookup, var_comp, parse_ctx, progress_f,
+               debug_f):
     self.pool = pool
     self.ev = ev
     self.comp_lookup = comp_lookup
     # This can happen in any position, with any command
     self.var_comp = var_comp
     self.parse_ctx = parse_ctx
+    self.progress_f = progress_f
+    self.debug_f = debug_f
 
     self.parser = DummyParser()  # TODO: remove
 
-  def Matches(self, buf, status_out):
+  def Matches(self, buf):
     arena = alloc.SideArena('<completion>')
     w_parser, c_parser = self.parse_ctx.MakeParserForCompletion(buf, arena)
     comp_type, prefix, comp_words = _GetCompletionType(
-        w_parser, c_parser, self.ev, status_out)
+        w_parser, c_parser, self.ev, self.debug_f)
 
     comp_type, prefix, comp_words = _GetCompletionType1(self.parser, buf)
 
@@ -642,7 +644,7 @@ def Matches(self, buf, status_out):
     else:
       raise AssertionError(comp_type)
 
-    status_out.Write(0, 'Completing %r ... (Ctrl-C to cancel)', buf)
+    self.progress_f.Write('Completing %r ... (Ctrl-C to cancel)', buf)
     start_time = time.time()
 
     index = len(comp_words) - 1  # COMP_CWORD -1 when it's empty
@@ -653,13 +655,13 @@ def Matches(self, buf, status_out):
       i += 1
       elapsed = time.time() - start_time
       plural = '' if i == 1 else 'es'
-      status_out.Write(0,
+      self.progress_f.Write(
           '... %d match%s for %r in %.2f seconds (Ctrl-C to cancel)', i,
           plural, buf, elapsed)
 
     elapsed = time.time() - start_time
     plural = '' if i == 1 else 'es'
-    status_out.Write(0,
+    self.progress_f.Write(
         'Found %d match%s for %r in %.2f seconds', i,
         plural, buf, elapsed)
 
@@ -670,10 +672,10 @@ def Matches(self, buf, status_out):
 
 
 class ReadlineCompleter(object):
-  def __init__(self, readline_mod, root_comp, status_out, debug=False):
+  def __init__(self, readline_mod, root_comp, debug_f, debug=False):
     self.readline_mod = readline_mod
     self.root_comp = root_comp
-    self.status_out = status_out
+    self.debug_f = debug_f
     self.debug = debug
 
     self.comp_iter = None  # current completion being processed
@@ -694,14 +696,14 @@ def _GetNextCompletion(self, state):
       end = self.readline_mod.get_endidx()
 
       if self.debug:
-        self.status_out.Write(0,
+        self.debug_f.log(
             'line: %r / begin - end: %d - %d, part: %r', buf, begin, end,
             buf[begin:end])
 
-      self.comp_iter = self.root_comp.Matches(buf, self.status_out)
+      self.comp_iter = self.root_comp.Matches(buf)
 
     if self.comp_iter is None:
-      self.status_out.Write(0, "ASSERT comp_iter shouldn't be None")
+      self.debug_f.log("ASSERT comp_iter shouldn't be None")
 
     try:
       next_completion = self.comp_iter.next()
@@ -715,12 +717,12 @@ def __call__(self, unused_word, state):
     # NOTE: The readline library tokenizes words.  We bypass that and use
     # get_line_buffer().  So we get 'for x in l' instead of just 'l'.
 
-    #self.status_out.Write(0, 'word %r state %s', unused_word, state)
+    #self.debug_f.log(0, 'word %r state %s', unused_word, state)
     try:
       return self._GetNextCompletion(state)
     except Exception as e:
       traceback.print_exc()
-      self.status_out.Write(0, 'Unhandled exception while completing: %s', e)
+      self.debug_f.log('Unhandled exception while completing: %s', e)
 
 
 def InitReadline(readline_mod, complete_cb):
@@ -758,19 +760,8 @@ def InitReadline(readline_mod, complete_cb):
   readline_mod.set_completer_delims(' ')
 
 
-class StatusOutput(object):
-  def __init__(self, status_lines, exec_opts):
-    self.status_lines = status_lines
-    self.exec_opts = exec_opts
-
-  def Write(self, index, msg, *args):
-    # Only line zero gets shown by default
-    if index == 0 or self.exec_opts.debug_completion:
-      self.status_lines[index].Write(msg, *args)
-
-
-def Init(readline_mod, pool, builtins, mem, funcs, comp_lookup, status_out, ev,
-         parse_ctx):
+def Init(readline_mod, pool, builtins, mem, funcs, comp_lookup, progress_f,
+         debug_f, ev, parse_ctx):
 
   aliases_action = WordsAction(['TODO:alias'])
   commands_action = ExternalCommandAction(mem)
@@ -797,9 +788,10 @@ def Init(readline_mod, pool, builtins, mem, funcs, comp_lookup, status_out, ev,
   comp_lookup.RegisterName('grep', C1)
 
   var_comp = VarAction(os.environ, mem)
-  root_comp = RootCompleter(pool, ev, comp_lookup, var_comp, parse_ctx)
+  root_comp = RootCompleter(pool, ev, comp_lookup, var_comp, parse_ctx,
+                            progress_f, debug_f)
 
-  complete_cb = ReadlineCompleter(readline_mod, root_comp, status_out)
+  complete_cb = ReadlineCompleter(readline_mod, root_comp, debug_f)
   InitReadline(readline_mod, complete_cb)
 
 

core/completion_test.py

@@ -5,19 +5,21 @@
 # You may obtain a copy of the License at
 #
 #   http://www.apache.org/licenses/LICENSE-2.0
+from __future__ import print_function
 """
 completion_test.py: Tests for completion.py
 """
-from __future__ import print_function
 
 import unittest
+import sys
 
 from core import alloc
 from core import cmd_exec_test
 from core import completion  # module under test
 from core import state
 from core import test_lib
 from core import ui
+from core import util
 from osh.meta import Id
 
 from osh.meta import ast
@@ -30,10 +32,10 @@
 
 C1 = completion.ChainedCompleter([A1])
 
-status_lines = [ui.TestStatusLine()] * 10  # A bunch of dummies
 mem = state.Mem('', [], {}, None)
 exec_opts = state.ExecOpts(mem, None)
-STATUS = completion.StatusOutput(status_lines, exec_opts)
+debug_f = util.DebugFile(sys.stdout)
+progress_f = ui.TestStatusLine()
 
 
 V1 = completion.WordsAction(['$var1', '$var2', '$another_var'])
@@ -123,34 +125,35 @@ def testRootCompleter(self):
     arena = pool.NewArena()
     parse_ctx = parse_lib.ParseContext(arena, {})
     var_comp = V1
-    r = completion.RootCompleter(pool, ev, comp_lookup, var_comp, parse_ctx)
+    r = completion.RootCompleter(pool, ev, comp_lookup, var_comp, parse_ctx,
+                                 progress_f, debug_f)
 
-    m = list(r.Matches('grep f', STATUS))
+    m = list(r.Matches('grep f'))
     self.assertEqual(['foo.py ', 'foo '], m)
 
-    m = list(r.Matches('grep g', STATUS))
+    m = list(r.Matches('grep g'))
     self.assertEqual([], m)
 
-    m = list(r.Matches('ls $v', STATUS))
+    m = list(r.Matches('ls $v'))
     self.assertEqual(['$var1 ', '$var2 '], m)
 
-    m = list(r.Matches('g', STATUS))
+    m = list(r.Matches('g'))
     self.assertEqual(['grep '], m)
 
     # Empty completer
-    m = list(r.Matches('', STATUS))
+    m = list(r.Matches(''))
     self.assertEqual(['grep ', 'sed ', 'test '], m)
 
     # Test compound commands. These PARSE
-    m = list(r.Matches('echo hi || grep f', STATUS))
-    m = list(r.Matches('echo hi; grep f', STATUS))
+    m = list(r.Matches('echo hi || grep f'))
+    m = list(r.Matches('echo hi; grep f'))
 
     # Brace -- does NOT parse
-    m = list(r.Matches('{ echo hi; grep f', STATUS))
+    m = list(r.Matches('{ echo hi; grep f'))
     # TODO: Test if/for/while/case/etc.
 
-    m = list(r.Matches('var=$v', STATUS))
-    m = list(r.Matches('local var=$v', STATUS))
+    m = list(r.Matches('var=$v'))
+    m = list(r.Matches('local var=$v'))
 
 
 def _TestGetCompletionType(buf):
@@ -159,7 +162,7 @@ def _TestGetCompletionType(buf):
   parse_ctx = parse_lib.ParseContext(arena, {})
   w_parser, c_parser = parse_ctx.MakeParserForCompletion(buf, arena)
   print('---', buf)
-  return completion._GetCompletionType(w_parser, c_parser, ev, STATUS)
+  return completion._GetCompletionType(w_parser, c_parser, ev, debug_f)
 
 
 f = _TestGetCompletionType

core/dev.py

@@ -5,7 +5,6 @@
 """
 
 import os
-from core import state
 from core import util
 
 # TODO: Move Tracer here.
@@ -37,14 +36,9 @@ def __init__(self, crash_dump_dir):
 
     # Things to dump:
     # Executor
-    #   functions, aliases, traps, completion hooks
+    #   functions, aliases, traps, completion hooks, fd_state
     #   dir_stack -- minor thing
     #
-    # state.Mem()
-    #  _ArgFrame
-    #  _StackFrame
-    # func names
-    #
     # debug info for the source?  Or does that come elsewhere?
     #
     # Yeah I think you sould have two separate files.
@@ -57,6 +51,8 @@ def __init__(self, crash_dump_dir):
     # One is constant at build time; the other is constant at runtime.
 
   def MaybeCollect(self, ex):
+    # TODO: Add error_with_loc here, and then save it in the JSON.
+
     if not self.do_collect:  # Either we already did it, or there is no file
       return
 
@@ -92,11 +88,14 @@ def MaybeDump(self, status):
     if not self.collected:
       return
 
+    # Other things we need: the reason for the crash!  _ErrorWithLocation is
+    # required I think.
     d = {
         'var_stack': self.var_stack,
         'argv_stack': self.argv_stack,
         'debug_stack': self.debug_stack,
     }
+
     path = os.path.join(self.crash_dump_dir, 'osh-crash-dump.json')
     with open(path, 'w') as f:
       import json

core/state.py

@@ -335,15 +335,17 @@ def Dump(self):
       if flags:
         cell_json['flags'] = flags
 
+      # For compactness, just put the value right in the cell.
       tag = cell.val.tag
       if tag == value_e.Undef:
-        val_json = {'type': 'Undef'}
+        cell_json['type'] = 'Undef'
       elif tag == value_e.Str:
-        val_json = {'type': 'Str', 'value': cell.val.s}
+        cell_json['type'] = 'Str'
+        cell_json['value'] = cell.val.s
       elif tag == value_e.StrArray:
-        val_json = {'type': 'StrArray', 'value': cell.val.strs}
+        cell_json['type'] = 'StrArray'
+        cell_json['value'] = cell.val.strs
 
-      cell_json['val'] = val_json
       vars_json[name] = cell_json
 
     return {