Merge pull request #1018 from gmmephisto/fix-find-editor

feat(utils): probe editors in system path

3 files changed, 79 insertions, 39 deletions

diff --git a/cmd2/utils.py b/cmd2/utils.py
index 45ca494c..ca07d23b 100644
--- a/cmd2/utils.py
+++ b/cmd2/utils.py
@@ -6,6 +6,7 @@ import collections.abc as collections_abc
 import functools
 import glob
 import inspect
+import itertools
 import os
 import re
 import subprocess
@@ -188,23 +189,6 @@ def namedtuple_with_defaults(typename: str, field_names: Union[str, List[str]],
     return T
 
 
-def which(exe_name: str) -> Optional[str]:
-    """Find the full path of a given executable on a Linux or Mac machine
-
-    :param exe_name: name of the executable to check, ie 'vi' or 'ls'
-    :return: a full path or None if exe not found
-    """
-    if sys.platform.startswith('win'):
-        return None
-
-    try:
-        exe_path = subprocess.check_output(['which', exe_name], stderr=subprocess.STDOUT).strip()
-        exe_path = exe_path.decode()
-    except subprocess.CalledProcessError:
-        exe_path = None
-    return exe_path
-
-
 def is_text_file(file_path: str) -> bool:
     """Returns if a file contains only ASCII or UTF-8 encoded text.
 
@@ -373,6 +357,24 @@ def expand_user_in_tokens(tokens: List[str]) -> None:
         tokens[index] = expand_user(tokens[index])
 
 
+def is_executable(path) -> bool:
+    """Return True if specified path is executable file, otherwise False."""
+    return os.path.isfile(path) and os.access(path, os.X_OK)
+
+
+def probe_editors() -> str:
+    """Find a favor editor in system path."""
+    editors = ['vim', 'vi', 'emacs', 'nano', 'pico', 'gedit', 'kate', 'subl', 'geany', 'atom']
+    paths = [p for p in os.getenv('PATH').split(os.path.pathsep) if not os.path.islink(p)]
+    for editor, path in itertools.product(editors, paths):
+        editor_path = os.path.join(path, editor)
+        if is_executable(editor_path):
+            break
+    else:
+        editor_path = None
+    return editor_path
+
+
 def find_editor() -> str:
     """Find a reasonable editor to use by default for the system that the cmd2 application is running on."""
     editor = os.environ.get('EDITOR')
@@ -380,10 +382,7 @@ def find_editor() -> str:
         if sys.platform[:3] == 'win':
             editor = 'notepad'
         else:
-            # Favor command-line editors first so we don't leave the terminal to edit
-            for editor in ['vim', 'vi', 'emacs', 'nano', 'pico', 'gedit', 'kate', 'subl', 'geany', 'atom']:
-                if which(editor):
-                    break
+            editor = probe_editors()
     return editor
 
 
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index 554f2ba7..2f24f4d7 100755
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -1275,24 +1275,6 @@ optional arguments:
   -s, --shout  N00B EMULATION MODE
 """
 
-@pytest.mark.skipif(sys.platform.startswith('win'),
-                    reason="utils.which function only used on Mac and Linux")
-def test_which_editor_good():
-    editor = cmd2.Cmd.DEFAULT_EDITOR
-    path = utils.which(editor)
-
-    # Assert that the editor was found because some editor should exist on all Mac and Linux systems
-    assert path
-
-@pytest.mark.skipif(sys.platform.startswith('win'),
-                    reason="utils.which function only used on Mac and Linux")
-def test_which_editor_bad():
-    nonexistent_editor = 'this_editor_does_not_exist.exe'
-    path = utils.which(nonexistent_editor)
-    # Assert that the non-existent editor wasn't found
-    assert path is None
-
-
 class MultilineApp(cmd2.Cmd):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, multiline_commands=['orate'], **kwargs)
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 2c94466c..ab3647e4 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -3,12 +3,18 @@
 """
 Unit testing for cmd2/utils.py module.
 """
+import os
 import signal
 import sys
 import time
 
 import pytest
 
+try:
+    import mock
+except ImportError:
+    from unittest import mock
+
 import cmd2.utils as cu
 from cmd2.constants import HORIZONTAL_ELLIPSIS
 
@@ -634,3 +640,56 @@ def test_str_to_bool_invalid():
 def test_str_to_bool_bad_input():
     with pytest.raises(ValueError):
         cu.str_to_bool(1)
+
+@mock.patch('cmd2.utils.probe_editors')
+def test_find_editor_specified(mock_probe_editors):
+    expected_editor = 'vim'
+    with mock.patch.dict(os.environ, {'EDITOR': expected_editor}):
+        editor = cu.find_editor()
+    assert editor == expected_editor
+    mock_probe_editors.assert_not_called()
+
+@pytest.mark.skipif(sys.platform.startswith('win'),
+                    reason="test 'find_editor' unix codepath")
+def test_find_editor_not_specified_unix():
+    expected_editor = 'vim'
+    with mock.patch.dict(os.environ, {'EDITOR': ''}):
+        with mock.patch(
+            'cmd2.utils.probe_editors',
+            return_value=expected_editor
+        ) as mock_probe_editors:
+            editor = cu.find_editor()
+    assert editor == expected_editor
+    mock_probe_editors.assert_called_once()
+
+@pytest.mark.skipif(not sys.platform.startswith('win'),
+                    reason="test 'find_editor' win codepath")
+def test_find_editor_not_specified_win():
+    expected_editor = 'notepad'
+    with mock.patch.dict(os.environ, {'EDITOR': ''}):
+        with mock.patch('cmd2.utils.probe_editors') as mock_probe_editors:
+            editor = cu.find_editor()
+    assert editor == expected_editor
+    mock_probe_editors.assert_not_called()
+
+@pytest.mark.skipif(sys.platform.startswith('win'),
+                    reason="test 'probe_editors' codepath")
+def test_probe_editors(tmpdir):
+    path = tmpdir.mkdir('bin')
+    vi_path = str(path.join('vi'))
+    with mock.patch.dict(os.environ, {'PATH': str(path)}):
+        editor = cu.probe_editors()
+    assert not editor
+
+    def mock_is_executable(p):
+        print(p, vi_path)
+        if p == vi_path:
+            return True
+
+    with mock.patch.dict(os.environ, {'PATH': str(path)}):
+        with mock.patch(
+            'cmd2.utils.is_executable',
+            mock_is_executable
+        ):
+            editor = cu.probe_editors()
+    assert editor == vi_path