Merge branch 'master' into set_prog

# Conflicts:
#	CHANGELOG.md

Author: tleonhardt

CHANGELOG.md

@@ -1,7 +1,12 @@
 ## 0.9.20 (TBD, 2019)
+* Bug Fixes
+    * Fixed bug where setting `use_ipython` to False removed ipy command from the entire `cmd2.Cmd` class instead of
+    just the instance being created
 * Enhancements
     * Send all startup script paths to run_script. Previously we didn't do this if the file was empty, but that
     showed no record of the run_script command in history. 
+    * Made it easier for developers to override `edit` command by having `do_history` no longer call `do_edit`. This
+    also removes the need to exclude `edit` command from history list.
     * It is no longer necessary to set the `prog` attribute of an argparser with subcommands. cmd2 now automatically
     sets the prog value of it and all its subparsers so that all usage statements contain the top level command name
     and not sys.argv[0].

cmd2/cmd2.py

@@ -390,10 +390,10 @@ def __init__(self, completekey: str = 'tab', stdin=None, stdout=None, *,
         :param shortcuts: dictionary containing shortcuts for commands. If not supplied, then defaults to
                           constants.DEFAULT_SHORTCUTS.
         """
-        # If use_ipython is False, make sure the do_ipy() method doesn't exit
+        # If use_ipython is False, make sure the ipy command isn't available in this instance
         if not use_ipython:
             try:
-                del Cmd.do_ipy
+                self.do_ipy = None
             except AttributeError:
                 pass
 
@@ -455,7 +455,7 @@ def __init__(self, completekey: str = 'tab', stdin=None, stdout=None, *,
         self._initialize_history(persistent_history_file)
 
         # Commands to exclude from the history command
-        self.exclude_from_history = '''history edit eof'''.split()
+        self.exclude_from_history = ['eof', 'history']
 
         # Dictionary of macro names and their values
         self.macros = dict()
@@ -3542,14 +3542,9 @@ def do_history(self, args: argparse.Namespace) -> Optional[bool]:
                     else:
                         fobj.write('{}\n'.format(command.raw))
             try:
-                # Handle potential edge case where the temp file needs to be quoted on the command line
-                quoted_fname = utils.quote_string(fname)
-
-                # noinspection PyTypeChecker
-                self.do_edit(quoted_fname)
-
+                self._run_editor(fname)
                 # noinspection PyTypeChecker
-                self.do_run_script(quoted_fname)
+                self.do_run_script(utils.quote_string(fname))
             finally:
                 os.remove(fname)
         elif args.output_file:
@@ -3741,25 +3736,33 @@ def _generate_transcript(self, history: List[Union[HistoryItem, str]], transcrip
             msg = '{} {} saved to transcript file {!r}'
             self.pfeedback(msg.format(commands_run, plural, transcript_file))
 
-    edit_description = ("Edit a file in a text editor\n"
+    edit_description = ("Run a text editor and optionally open a file with it\n"
                         "\n"
                         "The editor used is determined by a settable parameter. To set it:\n"
                         "\n"
                         "  set editor (program-name)")
 
     edit_parser = Cmd2ArgumentParser(description=edit_description)
     edit_parser.add_argument('file_path', nargs=argparse.OPTIONAL,
-                             help="path to a file to open in editor", completer_method=path_complete)
+                             help="optional path to a file to open in editor", completer_method=path_complete)
 
     @with_argparser(edit_parser)
     def do_edit(self, args: argparse.Namespace) -> None:
-        """Edit a file in a text editor"""
+        """Run a text editor and optionally open a file with it"""
+        self._run_editor(args.file_path)
+
+    def _run_editor(self, file_path: Optional[str]) -> None:
+        """
+        Run a text editor and optionally open a file with it
+        :param file_path: optional path of the file to edit
+        :raises EnvironmentError if self.editor is not set
+        """
         if not self.editor:
             raise EnvironmentError("Please use 'set editor' to specify your text editing program of choice.")
 
         command = utils.quote_string(os.path.expanduser(self.editor))
-        if args.file_path:
-            command += " " + utils.quote_string(os.path.expanduser(args.file_path))
+        if file_path:
+            command += " " + utils.quote_string(os.path.expanduser(file_path))
 
         # noinspection PyTypeChecker
         self.do_shell(command)

tests/test_history.py

@@ -476,9 +476,9 @@ def test_history_edit(base_app, monkeypatch):
     # going to call it due to the mock
     base_app.editor = 'fooedit'
 
-    # Mock out the edit call so we don't actually open an editor
-    edit_mock = mock.MagicMock(name='do_edit')
-    monkeypatch.setattr("cmd2.Cmd.do_edit", edit_mock)
+    # Mock out the _run_editor call so we don't actually open an editor
+    edit_mock = mock.MagicMock(name='_run_editor')
+    monkeypatch.setattr("cmd2.Cmd._run_editor", edit_mock)
 
     # Mock out the run_script call since the mocked edit won't produce a file
     run_script_mock = mock.MagicMock(name='do_run_script')
@@ -590,17 +590,6 @@ def test_base_help_history(base_app):
     assert out == normalize(HELP_HISTORY)
 
 def test_exclude_from_history(base_app, monkeypatch):
-    # Set a fake editor just to make sure we have one.  We aren't
-    # really going to call it due to the mock
-    base_app.editor = 'fooedit'
-
-    # Mock out the subprocess.Popen call so we don't actually open an editor
-    m = mock.MagicMock(name='Popen')
-    monkeypatch.setattr("subprocess.Popen", m)
-
-    # Run edit command
-    run_cmd(base_app, 'edit')
-
     # Run history command
     run_cmd(base_app, 'history')