Merge pull request #228 from python-cmd2/edit_index_bug

Fixed a bug when edit is passed an integer outside the range of history indices

Author: tleonhardt

cmd2.py

@@ -1026,7 +1026,7 @@ def pseudo_raw_input(self, prompt):
                 else:
                     line = sm.input()
                     if self.echo:
-                        sys.stdout.write('{}{}\n'.format(safe_prompt,line))
+                        sys.stdout.write('{}{}\n'.format(safe_prompt, line))
             except EOFError:
                 line = 'eof'
         else:
@@ -1669,10 +1669,29 @@ def do_edit(self, arg, opts=None):
         filename = None
         if arg and arg[0]:
             try:
-                history_item = self._last_matching(int(arg[0]))
+                # Try to convert argument to an integer
+                history_idx = int(arg[0])
             except ValueError:
+                # Argument passed is not convertible to an integer, so treat it as a file path
                 filename = arg[0]
                 history_item = ''
+            else:
+                # Argument passed IS convertible to an integer, so treat it as a history index
+
+                # Save off original index for pringing
+                orig_indx = history_idx
+
+                # Convert negative index into equivalent positive one
+                if history_idx < 0:
+                    history_idx += len(self.history) + 1
+
+                # Make sure the index is actually within the history
+                if 1 <= history_idx <= len(self.history):
+                    history_item = self._last_matching(history_idx)
+                else:
+                    self.perror('index {!r} does not exist within the history'.format(orig_indx), traceback_war=False)
+                    return
+
         else:
             try:
                 history_item = self.history[-1]
@@ -1817,7 +1836,7 @@ def do_load(self, file_path):
             # command queue. Add an "end of script (eos)" command to cleanup the
             # self._script_dir list when done. Specify file encoding in Python
             # 3, but Python 2 doesn't allow that argument to open().
-            kwargs = {'encoding' : 'utf-8'} if six.PY3 else {}
+            kwargs = {'encoding': 'utf-8'} if six.PY3 else {}
             with open(expanded_path, **kwargs) as target:
                 self.cmdqueue = target.read().splitlines() + ['eos'] + self.cmdqueue
         except IOError as e:
@@ -2368,8 +2387,6 @@ def _test_transcript(self, fname, transcript):
 
     def _transform_transcript_expected(self, s):
         """parse the string with slashed regexes into a valid regex"""
-        slash = '/'
-        backslash = '\\'
         regex = ''
         start = 0
 
@@ -2400,7 +2417,8 @@ def _transform_transcript_expected(self, s):
                     break
         return regex
 
-    def _escaped_find(self, regex, s, start, in_regex):
+    @staticmethod
+    def _escaped_find(regex, s, start, in_regex):
         """
         Find the next slash in {s} after {start} that is not preceded by a backslash.
 
@@ -2446,7 +2464,7 @@ def _escaped_find(self, regex, s, start, in_regex):
                 else:
                     # slash is not escaped, this is what we are looking for
                     break
-        return (regex, pos, start)
+        return regex, pos, start
 
     def tearDown(self):
         if self.cmdapp:

tests/test_cmd2.py

@@ -812,7 +812,29 @@ def test_edit_file_with_spaces(base_app, request, monkeypatch):
     # We think we have an editor, so should expect a system call
     m.assert_called_once_with('"{}" "{}"'.format(base_app.editor, filename))
 
-def test_edit_number(base_app, monkeypatch):
+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)
+
+    # Run help command just so we have a command in history
+    run_cmd(base_app, 'help')
+
+    run_cmd(base_app, 'edit')
+
+    # We have an editor, so should expect a system call
+    m.assert_called_once()
+
+def test_edit_empty_history(base_app, capsys):
+    run_cmd(base_app, 'edit')
+    out, err = capsys.readouterr()
+    assert out == ''
+    assert err == 'ERROR: edit must be called with argument if history is empty\n'
+
+def test_edit_valid_positive_number(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'
 
@@ -828,7 +850,7 @@ def test_edit_number(base_app, monkeypatch):
     # We have an editor, so should expect a system call
     m.assert_called_once()
 
-def test_edit_blank(base_app, monkeypatch):
+def test_edit_valid_negative_number(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'
 
@@ -839,16 +861,42 @@ def test_edit_blank(base_app, monkeypatch):
     # Run help command just so we have a command in history
     run_cmd(base_app, 'help')
 
-    run_cmd(base_app, 'edit')
+    run_cmd(base_app, 'edit "-1"')
 
     # We have an editor, so should expect a system call
     m.assert_called_once()
 
-def test_edit_empty_history(base_app, capsys):
-    run_cmd(base_app, 'edit')
-    out, err = capsys.readouterr()
-    assert out == ''
-    assert err == 'ERROR: edit must be called with argument if history is empty\n'
+def test_edit_invalid_positive_number(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)
+
+    # Run help command just so we have a command in history
+    run_cmd(base_app, 'help')
+
+    run_cmd(base_app, 'edit 23')
+
+    # History index is invalid, so should expect a system call
+    m.assert_not_called()
+
+def test_edit_invalid_negative_number(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)
+
+    # Run help command just so we have a command in history
+    run_cmd(base_app, 'help')
+
+    run_cmd(base_app, 'edit "-23"')
+
+    # History index is invalid, so should expect a system call
+    m.assert_not_called()
 
 
 def test_base_py_interactive(base_app):