Repository: python-cmd2/cmd2
Pull request #1100 (2021-04-28 14:43:04)

Updated Ctrl-C behavior
Changed default behavior of runcmds_plus_hooks() to not stop when Ctrl-C is pressed and instead run the next command in its list.

This means `run_script` will still stop upon Ctrl-C, but running commands from history and startup commands will act like the main command loop in that Ctrl-C will just move to the next command in the list.

Stopping a shell command with Ctrl-C now raises a KeyboardInterrupt to support stopping a text script which ran the shell command.

Removed `cmd2.Cmd.quit_on_sigint`.

codecov[bot]: # [Codecov](https://codecov.io/gh/python-cmd2/cmd2/pull/1100?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2) Report
> Merging [#1100](https://codecov.io/gh/python-cmd2/cmd2/pull/1100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2) (9dc0186) into [master](https://codecov.io/gh/python-cmd2/cmd2/commit/52aad8b80a619b13a67f9306b621df9c0e02bacf?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2) (52aad8b) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/python-cmd2/cmd2/pull/1100/graphs/tree.svg?width=650&height=150&src=pr&token=eks2PUwa6A&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2)](https://codecov.io/gh/python-cmd2/cmd2/pull/1100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2)

```diff
@@           Coverage Diff           @@
##           master    #1100   +/-   ##
=======================================
  Coverage   98.37%   98.37%           
=======================================
  Files          22       22           
  Lines        4736     4736           
=======================================
  Hits         4659     4659           
  Misses         77       77           
```


| [Impacted Files](https://codecov.io/gh/python-cmd2/cmd2/pull/1100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2) | Coverage Δ | |
|---|---|---|
| [cmd2/cmd2.py](https://codecov.io/gh/python-cmd2/cmd2/pull/1100/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2#diff-Y21kMi9jbWQyLnB5) | `98.15% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/python-cmd2/cmd2/pull/1100?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/python-cmd2/cmd2/pull/1100?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2). Last update [52aad8b...9dc0186](https://codecov.io/gh/python-cmd2/cmd2/pull/1100?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-cmd2).


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 11992122..3e6ca9cc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,6 +28,9 @@
       of this parameter.
     * Removed ability to run Python commands from the command line with `py`. Now `py` takes no arguments
       and just opens an interactive Python shell.
+    * Changed default behavior of `runcmds_plus_hooks()` to not stop when Ctrl-C is pressed and instead
+      run the next command in its list.
+    * Removed `cmd2.Cmd.quit_on_sigint` flag, which when `True`, quit the application when Ctrl-C was pressed at the prompt.
 * Enhancements
     * Added support for custom tab completion and up-arrow input history to `cmd2.Cmd2.read_input`.
       See [read_input.py](https://github.com/python-cmd2/cmd2/blob/master/examples/read_input.py)
@@ -47,6 +50,9 @@
     * Added `include_py` keyword parameter to `cmd2.Cmd.__init__()`. If `False`, then the `py` command will
       not be available. Defaults to `False`. `run_pyscript` is not affected by this parameter.
     * Made the amount of space between columns in a SimpleTable configurable
+    * On POSIX systems, shell commands and processes being piped to are now run in the user's preferred shell
+      instead of /bin/sh. The preferred shell is obtained by reading the SHELL environment variable. If that
+      doesn't exist or is empty, then /bin/sh is used.
 
 ## 1.5.0 (January 31, 2021)
 * Bug Fixes
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index e5dcca83..418f8a4f 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -293,7 +293,6 @@ class Cmd(cmd.Cmd):
 
         # Attributes which should NOT be dynamically settable via the set command at runtime
         self.default_to_shell = False  # Attempt to run unrecognized commands as shell commands
-        self.quit_on_sigint = False  # Ctrl-C at the prompt will quit the program instead of just resetting prompt
         self.allow_redirection = allow_redirection  # Security setting to prevent redirection of stdout
 
         # Attributes which ARE dynamically settable via the set command at runtime
@@ -2114,10 +2113,10 @@ class Cmd(cmd.Cmd):
                 ansi.style_aware_write(sys.stdout, '\n' + err_str + '\n')
                 rl_force_redisplay()
             return None
-        except Exception as e:
+        except Exception as ex:
             # Insert a newline so the exception doesn't print in the middle of the command line being tab completed
             self.perror()
-            self.pexcept(e)
+            self.pexcept(ex)
             rl_force_redisplay()
             return None
 
@@ -2199,7 +2198,11 @@ class Cmd(cmd.Cmd):
 
         # Check if we are allowed to re-raise the KeyboardInterrupt
         if not self.sigint_protection:
-            raise KeyboardInterrupt("Got a keyboard interrupt")
+            self._raise_keyboard_interrupt()
+
+    def _raise_keyboard_interrupt(self) -> None:
+        """Helper function to raise a KeyboardInterrupt"""
+        raise KeyboardInterrupt("Got a keyboard interrupt")
 
     def precmd(self, statement: Union[Statement, str]) -> Statement:
         """Hook method executed just before the command is executed by
@@ -2405,7 +2408,7 @@ class Cmd(cmd.Cmd):
         cmds: Union[List[HistoryItem], List[str]],
         *,
         add_to_history: bool = True,
-        stop_on_keyboard_interrupt: bool = True,
+        stop_on_keyboard_interrupt: bool = False,
     ) -> bool:
         """
         Used when commands are being run in an automated fashion like text scripts or history replays.
@@ -2413,8 +2416,9 @@ class Cmd(cmd.Cmd):
 
         :param cmds: commands to run
         :param add_to_history: If True, then add these commands to history. Defaults to True.
-        :param stop_on_keyboard_interrupt: stop command loop if Ctrl-C is pressed instead of just
-                                           moving to the next command. Defaults to True.
+        :param stop_on_keyboard_interrupt: if True, then stop running contents of cmds if Ctrl-C is pressed instead of moving
+                                           to the next command in the list. This is used when the commands are part of a
+                                           group, like a text script, which should stop upon Ctrl-C. Defaults to False.
         :return: True if running of commands should stop
         """
         for line in cmds:
@@ -2429,9 +2433,9 @@ class Cmd(cmd.Cmd):
                     line, add_to_history=add_to_history, raise_keyboard_interrupt=stop_on_keyboard_interrupt
                 ):
                     return True
-            except KeyboardInterrupt as e:
+            except KeyboardInterrupt as ex:
                 if stop_on_keyboard_interrupt:
-                    self.perror(e)
+                    self.perror(ex)
                     break
 
         return False
@@ -2485,13 +2489,10 @@ class Cmd(cmd.Cmd):
                     nextline = '\n'
                     self.poutput(nextline)
                 line = f'{self._multiline_in_progress}{nextline}'
-            except KeyboardInterrupt as ex:
-                if self.quit_on_sigint:
-                    raise ex
-                else:
-                    self.poutput('^C')
-                    statement = self.statement_parser.parse('')
-                    break
+            except KeyboardInterrupt:
+                self.poutput('^C')
+                statement = self.statement_parser.parse('')
+                break
             finally:
                 self._at_continuation_prompt = False
 
@@ -2622,12 +2623,17 @@ class Cmd(cmd.Cmd):
             # Create pipe process in a separate group to isolate our signals from it. If a Ctrl-C event occurs,
             # our sigint handler will forward it only to the most recent pipe process. This makes sure pipe
             # processes close in the right order (most recent first).
-            kwargs = dict()
+            kwargs: Dict[str, Any] = dict()
             if sys.platform == 'win32':
                 kwargs['creationflags'] = subprocess.CREATE_NEW_PROCESS_GROUP
             else:
                 kwargs['start_new_session'] = True
 
+                # Attempt to run the pipe process in the user's preferred shell instead of the default behavior of using sh.
+                shell = os.environ.get("SHELL")
+                if shell:
+                    kwargs['executable'] = shell
+
             # For any stream that is a StdSim, we will use a pipe so we can capture its output
             proc = subprocess.Popen(  # type: ignore[call-overload]
                 statement.pipe_to,
@@ -2653,7 +2659,7 @@ class Cmd(cmd.Cmd):
                 new_stdout.close()
                 raise RedirectionError(f'Pipe process exited with code {proc.returncode} before command could run')
             else:
-                redir_saved_state.redirecting = True
+                redir_saved_state.redirecting = True  # type: ignore[unreachable]
                 cmd_pipe_proc_reader = utils.ProcReader(proc, cast(TextIO, self.stdout), sys.stderr)
                 sys.stdout = self.stdout = new_stdout
 
@@ -3057,12 +3063,9 @@ class Cmd(cmd.Cmd):
                 # Get commands from user
                 try:
                     line = self._read_command_line(self.prompt)
-                except KeyboardInterrupt as ex:
-                    if self.quit_on_sigint:
-                        raise ex
-                    else:
-                        self.poutput('^C')
-                        line = ''
+                except KeyboardInterrupt:
+                    self.poutput('^C')
+                    line = ''
 
                 # Run the command along with all associated pre and post hooks
                 stop = self.onecmd_plus_hooks(line)
@@ -3824,8 +3827,8 @@ class Cmd(cmd.Cmd):
                     orig_value = settable.get_value()
                     new_value = settable.set_value(utils.strip_quotes(args.value))
                 # noinspection PyBroadException
-                except Exception as e:
-                    self.perror(f"Error setting {args.param}: {e}")
+                except Exception as ex:
+                    self.perror(f"Error setting {args.param}: {ex}")
                 else:
                     self.poutput(f"{args.param} - was: {orig_value!r}\nnow: {new_value!r}")
                 return
@@ -3862,8 +3865,30 @@ class Cmd(cmd.Cmd):
     @with_argparser(shell_parser, preserve_quotes=True)
     def do_shell(self, args: argparse.Namespace) -> None:
         """Execute a command as if at the OS prompt"""
+        import signal
         import subprocess
 
+        kwargs: Dict[str, Any] = dict()
+
+        # Set OS-specific parameters
+        if sys.platform.startswith('win'):
+            # Windows returns STATUS_CONTROL_C_EXIT when application stopped by Ctrl-C
+            ctrl_c_ret_code = 0xC000013A
+        else:
+            # On POSIX, Popen() returns -SIGINT when application stopped by Ctrl-C
+            ctrl_c_ret_code = signal.SIGINT.value * -1
+
+            # On POSIX with shell=True, Popen() defaults to /bin/sh as the shell.
+            # sh reports an incorrect return code for some applications when Ctrl-C is pressed within that
+            # application (e.g. less). Since sh received the SIGINT, it sets the return code to reflect being
+            # closed by SIGINT even though less did not exit upon a Ctrl-C press. In the same situation, other
+            # shells like bash and zsh report the actual return code of less. Therefore we will try to run the
+            # user's preferred shell which most likely will be something other than sh. This also allows the user
+            # to run builtin commands of their preferred shell.
+            shell = os.environ.get("SHELL")
+            if shell:
+                kwargs['executable'] = shell
+
         # Create a list of arguments to shell
         tokens = [args.command] + args.command_args
 
@@ -3875,19 +3900,25 @@ class Cmd(cmd.Cmd):
         # still receive the SIGINT since it is in the same process group as us.
         with self.sigint_protection:
             # For any stream that is a StdSim, we will use a pipe so we can capture its output
-            proc = subprocess.Popen(
+            proc = subprocess.Popen(  # type: ignore[call-overload]
                 expanded_command,
                 stdout=subprocess.PIPE if isinstance(self.stdout, utils.StdSim) else self.stdout,  # type: ignore[unreachable]
                 stderr=subprocess.PIPE if isinstance(sys.stderr, utils.StdSim) else sys.stderr,  # type: ignore[unreachable]
                 shell=True,
+                **kwargs,
             )
 
-            proc_reader = utils.ProcReader(proc, cast(TextIO, self.stdout), sys.stderr)
+            proc_reader = utils.ProcReader(proc, cast(TextIO, self.stdout), sys.stderr)  # type: ignore[arg-type]
             proc_reader.wait()
 
             # Save the return code of the application for use in a pyscript
             self.last_result = proc.returncode
 
+            # If the process was stopped by Ctrl-C, then inform the caller by raising a KeyboardInterrupt.
+            # This is to support things like stop_on_keyboard_interrupt in run_cmds_plus_hooks().
+            if proc.returncode == ctrl_c_ret_code:
+                self._raise_keyboard_interrupt()
+
     @staticmethod
     def _reset_py_display() -> None:
         """
@@ -4544,8 +4575,8 @@ class Cmd(cmd.Cmd):
                 # then run the command and let the output go into our buffer
                 try:
                     stop = self.onecmd_plus_hooks(history_item, raise_keyboard_interrupt=True)
-                except KeyboardInterrupt as e:
-                    self.perror(e)
+                except KeyboardInterrupt as ex:
+                    self.perror(ex)
                     stop = True
 
                 commands_run += 1
@@ -4684,7 +4715,7 @@ class Cmd(cmd.Cmd):
             if args.transcript:
                 self._generate_transcript(script_commands, os.path.expanduser(args.transcript))
             else:
-                return self.runcmds_plus_hooks(script_commands)
+                return self.runcmds_plus_hooks(script_commands, stop_on_keyboard_interrupt=True)
 
         finally:
             with self.sigint_protection:
diff --git a/cmd2/utils.py b/cmd2/utils.py
index bb3d1a65..cbbd1800 100644
--- a/cmd2/utils.py
+++ b/cmd2/utils.py
@@ -601,8 +601,8 @@ class ProcReader:
         import signal
 
         if sys.platform.startswith('win'):
-            # cmd2 started the Windows process in a new process group. Therefore
-            # a CTRL_C_EVENT can't be sent to it. Send a CTRL_BREAK_EVENT instead.
+            # cmd2 started the Windows process in a new process group. Therefore we must send
+            # a CTRL_BREAK_EVENT since CTRL_C_EVENT signals cannot be generated for process groups.
             self._proc.send_signal(signal.CTRL_BREAK_EVENT)
         else:
             # Since cmd2 uses shell=True in its Popen calls, we need to send the SIGINT to
diff --git a/docs/features/initialization.rst b/docs/features/initialization.rst
index ab792c2c..e3acf859 100644
--- a/docs/features/initialization.rst
+++ b/docs/features/initialization.rst
@@ -146,8 +146,6 @@ override:
   everything available with **self_in_py**)
 - **quiet**: if ``True`` then completely suppress nonessential output (Default:
   ``False``)
-- **quit_on_sigint**: if ``True`` Ctrl-C at the prompt will quit the program
-  instead of just resetting prompt
 - **settable**: dictionary that controls which of these instance attributes
   are settable at runtime using the *set* command
 - **timing**: if ``True`` display execution time for each command (Default:
diff --git a/docs/features/misc.rst b/docs/features/misc.rst
index 3825065f..c2f14b5b 100644
--- a/docs/features/misc.rst
+++ b/docs/features/misc.rst
@@ -99,21 +99,3 @@ method be called.
   (Cmd) my dog has fleas
   sh: my: not found
   *** Unknown syntax: my dog has fleas
-
-
-Quit on SIGINT
---------------
-
-On many shells, SIGINT (most often triggered by the user pressing Ctrl+C)
-while at the prompt only cancels the current line, not the entire command
-loop. By default, a ``cmd2`` application matches this behavior. However, if
-``quit_on_sigint`` is set to ``True``, the command loop will quit instead.
-
-::
-
-  (Cmd) typing a comma^C
-  (Cmd)
-
-.. warning::
-    The default SIGINT behavior will only function properly if **cmdloop** is running
-    in the main thread.


Test patch:
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index b09141c9..cfa30830 100755
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -7,6 +7,7 @@ import argparse
 import builtins
 import io
 import os
+import signal
 import sys
 import tempfile
 from code import (
@@ -422,19 +423,19 @@ def test_runcmds_plus_hooks_ctrl_c(base_app, capsys):
 
     setattr(base_app, 'do_keyboard_interrupt', types.MethodType(do_keyboard_interrupt, base_app))
 
-    # Default behavior is to stop command loop on Ctrl-C
+    # Default behavior is to not stop runcmds_plus_hooks() on Ctrl-C
     base_app.history.clear()
     base_app.runcmds_plus_hooks(['help', 'keyboard_interrupt', 'shortcuts'])
     out, err = capsys.readouterr()
-    assert err.startswith("Interrupting this command")
-    assert len(base_app.history) == 2
+    assert not err
+    assert len(base_app.history) == 3
 
-    # Ctrl-C should not stop command loop in this case
+    # Ctrl-C should stop runcmds_plus_hooks() in this case
     base_app.history.clear()
-    base_app.runcmds_plus_hooks(['help', 'keyboard_interrupt', 'shortcuts'], stop_on_keyboard_interrupt=False)
+    base_app.runcmds_plus_hooks(['help', 'keyboard_interrupt', 'shortcuts'], stop_on_keyboard_interrupt=True)
     out, err = capsys.readouterr()
-    assert not err
-    assert len(base_app.history) == 3
+    assert err.startswith("Interrupting this command")
+    assert len(base_app.history) == 2
 
 
 def test_relative_run_script(base_app, request):
@@ -902,6 +903,22 @@ def test_stty_sane(base_app, monkeypatch):
         m.assert_called_once_with(['stty', 'sane'])
 
 
+def test_sigint_handler(base_app):
+    # No KeyboardInterrupt should be raised when using sigint_protection
+    with base_app.sigint_protection:
+        base_app.sigint_handler(signal.SIGINT, 1)
+
+    # Without sigint_protection, a KeyboardInterrupt is raised
+    with pytest.raises(KeyboardInterrupt):
+        base_app.sigint_handler(signal.SIGINT, 1)
+
+
+def test_raise_keyboard_interrupt(base_app):
+    with pytest.raises(KeyboardInterrupt) as excinfo:
+        base_app._raise_keyboard_interrupt()
+    assert 'Got a keyboard interrupt' in str(excinfo.value)
+
+
 class HookFailureApp(cmd2.Cmd):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
@@ -945,36 +962,13 @@ def say_app():
     return app
 
 
-def test_interrupt_quit(say_app):
-    say_app.quit_on_sigint = True
-
+def test_ctrl_c_at_prompt(say_app):
     # Mock out the input call so we don't actually wait for a user's response on stdin
     m = mock.MagicMock(name='input')
     m.side_effect = ['say hello', KeyboardInterrupt(), 'say goodbye', 'eof']
     builtins.input = m
 
-    try:
-        say_app.cmdloop()
-    except KeyboardInterrupt:
-        pass
-
-    # And verify the expected output to stdout
-    out = say_app.stdout.getvalue()
-    assert out == 'hello\n'
-
-
-def test_interrupt_noquit(say_app):
-    say_app.quit_on_sigint = False
-
-    # Mock out the input call so we don't actually wait for a user's response on stdin
-    m = mock.MagicMock(name='input')
-    m.side_effect = ['say hello', KeyboardInterrupt(), 'say goodbye', 'eof']
-    builtins.input = m
-
-    try:
-        say_app.cmdloop()
-    except KeyboardInterrupt:
-        pass
+    say_app.cmdloop()
 
     # And verify the expected output to stdout
     out = say_app.stdout.getvalue()