Interactive loop respects the 'exit' builtin.

Fixes issue #87. I had changed the exit builtin to control flow, and the interactive loop just swallowed its status. Now we have a new function that returns (status, is_control_flow).
oilshell · Mar 5, 2018 · 631d3c559c1230269aa77bb0a7ff7b6c1f63940a · 631d3c5
1 parent c001d41
commit 631d3c559c1230269aa77bb0a7ff7b6c1f63940a
Unified Split

Showing with 45 additions and 25 deletions.

+7 −5 bin/oil.py

+26 −16 core/cmd_exec.py

+5 −0 test/common.sh

+7 −4 test/smoke.sh
diff --git a/bin/oil.py b/bin/oil.py
@@ -98,6 +98,7 @@ def InteractiveLoop(opts, ex, c_parser, w_parser, line_reader):
  else:
    ast_f = None

+  status = 0
  while True:
    try:
      w = c_parser.Peek()
@@ -126,7 +127,9 @@ def InteractiveLoop(opts, ex, c_parser, w_parser, line_reader):
      if ast_f:
        ast_lib.PrettyPrint(node)

-      status = ex.Execute(node)
+      status, is_control_flow = ex.ExecuteAndCatch(node)
+      if is_control_flow:  # exit or return
+        break

      if opts.print_status:
        print('STATUS', repr(status))
@@ -141,6 +144,8 @@ def InteractiveLoop(opts, ex, c_parser, w_parser, line_reader):
    w_parser.Reset()
    c_parser.Reset()

+  return status
+

 # bash --noprofile --norc uses 'bash-4.3$ '
 OSH_PS1 = 'osh$ '
@@ -298,10 +303,7 @@ def OshMain(argv0, argv, login_shell):
      completion.Init(pool, builtin.BUILTIN_DEF, mem, funcs, comp_lookup,
                      status_out, ev)

-    InteractiveLoop(opts, ex, c_parser, w_parser, line_reader)
-    # TODO: status should be last command.  Start bash, type "f() { return 33;
-    # }; f"
-    status = 0
+    return InteractiveLoop(opts, ex, c_parser, w_parser, line_reader)
  else:
    # Parse the whole thing up front
    #print('Parsing file')
diff --git a/core/cmd_exec.py b/core/cmd_exec.py
@@ -1166,9 +1166,32 @@ def _ExecuteList(self, children):
      status = self._Execute(child)  # last status wins
    return status

-  def Execute(self, node, fork_external=True, run_exit_trap=False):
+  def ExecuteAndCatch(self, node, fork_external=True):
+    """Used directly by the interactive loop."""
+    is_control_flow = False
+    try:
+      status = self._Execute(node, fork_external=fork_external)
+    except _ControlFlow as e:
+      # Return at top level is OK, unlike in bash.
+      if e.IsReturn() or e.IsExit():
+        is_control_flow = True
+        status = e.StatusCode()
+      else:
+        raise  # Invalid
+    except util.FatalRuntimeError as e:
+      ui.PrettyPrintError(e, self.arena)
+      print('osh failed: %s' % e.UserErrorString(), file=sys.stderr)
+      status = e.exit_status if e.exit_status is not None else 1
+      # TODO: dump self.mem if requested.  Maybe speify with OIL_DUMP_PREFIX.
+
+    # Other exceptions: SystemExit for sys.exit()
+    return status, is_control_flow
+
+  def Execute(self, node, fork_external=True):
    """Execute a subprogram, handling _ControlFlow and fatal exceptions.

+    This is just like ExecuteAndCatch, but we don't return is_control_flow.
+
    Callers:
    - SubProgramThunk for pipelines, subshell, command sub, process sub
    - .oilrc
@@ -1188,21 +1211,8 @@ def Execute(self, node, fork_external=True, run_exit_trap=False):
    Returns:
      status: numeric exit code
    """
-    try:
-      status = self._Execute(node, fork_external=fork_external)
-    except _ControlFlow as e:
-      # Return at top level is OK, unlike in bash.
-      if e.IsReturn() or e.IsExit():
-        status = e.StatusCode()
-      else:
-        raise
-    except util.FatalRuntimeError as e:
-      ui.PrettyPrintError(e, self.arena)
-      print('osh failed: %s' % e.UserErrorString(), file=sys.stderr)
-      status = e.exit_status if e.exit_status is not None else 1
-      # TODO: dump self.mem if requested.  Maybe speify with OIL_DUMP_PREFIX.
-
-    # Other exceptions: SystemExit for sys.exit()
+    # Ignore is_control_flow
+    status, _ = self.ExecuteAndCatch(node, fork_external=fork_external)
    return status

  def ExecuteAndRunExitTrap(self, node):
diff --git a/test/common.sh b/test/common.sh
@@ -28,6 +28,11 @@ fail() {
  exit 1
 }

+# NOTE: Could use BASH_SOURCE and so forth for a better error message.
+assert() {
+  test "$@" || die "'$@' failed"
+}
+
 run-task-with-status() {
  local out_file=$1
  shift
diff --git a/test/smoke.sh b/test/smoke.sh
@@ -11,10 +11,6 @@ set -o errexit

 source test/common.sh

-assert() {
-  test "$@" || die "$@ failed"
-}
-
 ast() {
  bin/osh -n -c 'echo hi'
  bin/osh -n --ast-format text -c 'echo hi'
@@ -140,11 +136,18 @@ parse-errors() {
  #_error-case 'echo ${'
 }

+exit-builtin-interactive() {
+  set +o errexit
+  echo 'echo one; exit 42; echo two' | bin/osh -i
+  assert $? -eq 42
+}
+
 readonly -a PASSING=(
  ast
  osh-file
  osh-stdin
  osh-interactive
+  exit-builtin-interactive
  help
 )