Updated utils.find_editor() to include more Windows editors

2 files changed, 32 insertions, 71 deletions

diff --git a/cmd2/utils.py b/cmd2/utils.py
index ca07d23b..06dd69b5 100644
--- a/cmd2/utils.py
+++ b/cmd2/utils.py
@@ -357,32 +357,30 @@ 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."""
+def find_editor() -> Optional[str]:
+    """
+    Used to set cmd2.Cmd.DEFAULT_EDITOR. If EDITOR env variable is set, that will be used.
+    Otherwise the function will look for a known editor in directories specified by PATH env variable.
+    :return: Default editor or None
+    """
     editor = os.environ.get('EDITOR')
     if not editor:
         if sys.platform[:3] == 'win':
-            editor = 'notepad'
+            editors = ['code.cmd', 'notepad++.exe', 'notepad.exe']
         else:
-            editor = probe_editors()
+            editors = ['vim', 'vi', 'emacs', 'nano', 'pico', 'gedit', 'kate', 'code', '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 os.path.isfile(editor_path) and os.access(editor_path, os.X_OK):
+                if sys.platform[:3] == 'win':
+                    # Remove extension from Windows file names
+                    editor = os.path.splitext(editor)[0]
+                break
+        else:
+            editor = None
+
     return editor
 
 
diff --git a/tests/test_utils.py b/tests/test_utils.py
index ab3647e4..f4a082ae 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -641,55 +641,18 @@ 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'
+def test_find_editor_specified():
+    expected_editor = os.path.join('fake_dir', 'editor')
     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
+
+def test_find_editor_not_specified():
+    # Use existing path env setting. Something in the editor list should be found.
+    editor = cu.find_editor()
+    assert editor
+
+    # Overwrite path env setting with invalid path. No editor should be found.
+    with mock.patch.dict(os.environ, {'PATH': 'fake_dir'}):
+        editor = cu.find_editor()
+    assert editor is None