From fa5e64d675ad390d1e5511a2ee698f177637b376 Mon Sep 17 00:00:00 2001
From: Kevin Van Brunt <kmvanbrunt@gmail.com>
Date: Thu, 21 Feb 2019 20:08:31 -0500
Subject: [PATCH 1/2] Removed os.system in favor of do_shell

---
 cmd2/cmd2.py       | 11 +++++------
 tests/test_cmd2.py | 36 +++++++++++++++++-------------------
 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 65435d6b8..d6da8b2b0 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -3248,7 +3248,7 @@ def do_history(self, args: argparse.Namespace) -> None:
                 for command in history:
                     fobj.write('{}\n'.format(command))
             try:
-                os.system('"{}" "{}"'.format(self.editor, fname))
+                self.do_edit(fname)
                 self.do_load(fname)
             except Exception:
                 raise
@@ -3356,12 +3356,11 @@ def do_edit(self, args: argparse.Namespace) -> None:
         if not self.editor:
             raise EnvironmentError("Please use 'set editor' to specify your text editing program of choice.")
 
-        editor = utils.quote_string_if_needed(self.editor)
+        command = utils.quote_string_if_needed(self.editor)
         if args.file_path:
-            expanded_path = utils.quote_string_if_needed(os.path.expanduser(args.file_path))
-            os.system('{} {}'.format(editor, expanded_path))
-        else:
-            os.system('{}'.format(editor))
+            command += " " + utils.quote_string_if_needed(args.file_path)
+
+        self.do_shell(command)
 
     @property
     def _current_script_dir(self) -> Optional[str]:
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index 09c4fa6ce..8b6035bd6 100644
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -459,15 +459,15 @@ def test_history_edit(base_app, monkeypatch):
     # going to call it due to the mock
     base_app.editor = 'fooedit'
 
-    # Mock out the os.system call so we don't actually open an editor
-    m = mock.MagicMock(name='system')
-    monkeypatch.setattr("os.system", m)
+    # 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 help command just so we have a command in history
     run_cmd(base_app, 'help')
     run_cmd(base_app, 'history -e 1')
 
-    # We have an editor, so should expect a system call
+    # We have an editor, so should expect a Popen call
     m.assert_called_once()
 
 def test_history_run_all_commands(base_app):
@@ -883,46 +883,44 @@ def test_edit_file(base_app, request, monkeypatch):
     base_app.editor = 'fooedit'
 
     # Mock out the os.system call so we don't actually open an editor
-    m = mock.MagicMock(name='system')
-    monkeypatch.setattr("os.system", m)
+    m = mock.MagicMock(name='Popen')
+    monkeypatch.setattr("subprocess.Popen", m)
 
     test_dir = os.path.dirname(request.module.__file__)
     filename = os.path.join(test_dir, 'script.txt')
 
     run_cmd(base_app, 'edit {}'.format(filename))
 
-    # We think we have an editor, so should expect a system call
-    m.assert_called_once_with('{} {}'.format(utils.quote_string_if_needed(base_app.editor),
-                                             utils.quote_string_if_needed(filename)))
+    # We think we have an editor, so should expect a Popen call
+    m.assert_called_once()
 
 def test_edit_file_with_spaces(base_app, request, 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 os.system call so we don't actually open an editor
-    m = mock.MagicMock(name='system')
-    monkeypatch.setattr("os.system", m)
+    # Mock out the subprocess.Popen call so we don't actually open an editor
+    m = mock.MagicMock(name='Popen')
+    monkeypatch.setattr("subprocess.Popen", m)
 
     test_dir = os.path.dirname(request.module.__file__)
     filename = os.path.join(test_dir, 'my commands.txt')
 
     run_cmd(base_app, 'edit "{}"'.format(filename))
 
-    # We think we have an editor, so should expect a system call
-    m.assert_called_once_with('{} {}'.format(utils.quote_string_if_needed(base_app.editor),
-                                             utils.quote_string_if_needed(filename)))
+    # We think we have an editor, so should expect a Popen call
+    m.assert_called_once()
 
 def test_edit_blank(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 os.system call so we don't actually open an editor
-    m = mock.MagicMock(name='system')
-    monkeypatch.setattr("os.system", m)
+    # 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_cmd(base_app, 'edit')
 
-    # We have an editor, so should expect a system call
+    # We have an editor, so should expect a Popen call
     m.assert_called_once()
 
 

From befeaafa59acd86fd5b467dc459e8df131bf03eb Mon Sep 17 00:00:00 2001
From: Kevin Van Brunt <kmvanbrunt@gmail.com>
Date: Thu, 21 Feb 2019 20:28:30 -0500
Subject: [PATCH 2/2] Updated change log

---
 CHANGELOG.md | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6d3b4c328..5b034faad 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,8 @@
-## 0.9.9 (TBD, 2019)
+## 0.9.9 (February 21, 2019)
 * Bug Fixes
     * Fixed bug where the ``set`` command was not tab completing from the current ``settable`` dictionary.
+* Enhancements
+    * Changed edit command to use do_shell() instead of calling os.system()
     
 ## 0.9.8 (February 06, 2019)
 * Bug Fixes