Merge pull request #795 from python-cmd2/history_edit

do_history no longer calls do_edit

Author: tleonhardt

CHANGELOG.md

@@ -5,6 +5,8 @@
 * 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.
 
 ## 0.9.19 (October 14, 2019)
 * Bug Fixes

cmd2/cmd2.py

@@ -433,7 +433,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()
@@ -3520,14 +3520,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:
@@ -3719,25 +3714,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')