Merged master into history branch and fixed merge conflicts

Author: tleonhardt

CHANGELOG.md

@@ -1,6 +1,12 @@
-## 0.9.9 (TBD, 2019)
+## 0.9.10 (February 22, 2019)
+* Bug Fixes
+    * Fixed unit test that hangs on Windows
+
+## 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

README.md

@@ -31,7 +31,7 @@ Main Features
 - Multi-line commands
 - Special-character command shortcuts (beyond cmd's `?` and `!`)
 - Command aliasing similar to bash `alias` command
-- Macros, which are similar to aliases, but can take arguments when called
+- Macros, which are similar to aliases, but they can contain argument placeholders
 - Ability to load commands at startup from an initialization script
 - Settable environment parameters
 - Parsing commands with arguments using `argparse`, including support for sub-commands

cmd2/cmd2.py

@@ -2408,7 +2408,7 @@ def macro_list(self, args: argparse.Namespace) -> None:
     # Top-level parser for macro
     macro_description = ("Manage macros\n"
                          "\n"
-                         "A macro is similar to an alias, but it can take arguments when called.")
+                         "A macro is similar to an alias, but it can contain argument placeholders.")
     macro_epilog = ("See also:\n"
                     "  alias")
     macro_parser = ACArgumentParser(description=macro_description, epilog=macro_epilog, prog='macro')
@@ -2420,7 +2420,7 @@ def macro_list(self, args: argparse.Namespace) -> None:
     macro_create_help = "create or overwrite a macro"
     macro_create_description = "Create or overwrite a macro"
 
-    macro_create_epilog = ("A macro is similar to an alias, but it can take arguments when called.\n"
+    macro_create_epilog = ("A macro is similar to an alias, but it can contain argument placeholders.\n"
                            "Arguments are expressed when creating a macro using {#} notation where {1}\n"
                            "means the first argument.\n"
                            "\n"
@@ -3247,7 +3247,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
@@ -3352,12 +3352,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(os.path.expanduser(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(os.path.expanduser(args.file_path))
+
+        self.do_shell(command)
 
     @property
     def _current_script_dir(self) -> Optional[str]:

docs/settingchanges.rst

@@ -75,10 +75,8 @@ Macros
 ======
 
 ``cmd2`` provides a feature that is similar to aliases called macros. The major difference between macros and aliases
-is that macros are intended to take arguments when called. These can be useful if you need to run a complex command
-frequently with different arguments that appear in various parts of the command.
-
-Arguments are expressed when creating a macro using {#} notation where {1} means the first argument.
+is that macros can contain argument placeholders. Arguments are expressed when creating a macro using {#} notation
+where {1} means the first argument.
 
 The following creates a macro called my_macro that expects two arguments:
 

setup.py

@@ -28,7 +28,7 @@
     - Multi-line commands
     - Special-character command shortcuts (beyond cmd's `?` and `!`)
     - Command aliasing similar to bash `alias` command
-    - Macros, which are similar to aliases, but can take arguments when called
+    - Macros, which are similar to aliases, but they can contain argument placeholders
     - Ability to load commands at startup from an initialization script
     - Settable environment parameters
     - Parsing commands with arguments using `argparse`, including support for sub-commands

tests/test_cmd2.py

@@ -678,47 +678,45 @@ def test_edit_file(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, '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()
 
 

tests/test_history.py

@@ -221,8 +221,8 @@ def test_history_edit(base_app, 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)
 
     # Run help command just so we have a command in history
     run_cmd(base_app, 'help')