From 3c4dc79e8ac53a67e507d77d2172a68a4723b7e7 Mon Sep 17 00:00:00 2001
From: Micah Jerome Ellison <micah.jerome.ellison@gmail.com>
Date: Sat, 16 Jan 2021 12:28:34 -0800
Subject: [PATCH] Fix inverted POSIX check, refactor os_compat, and add tests
 for it

---
 features/environment.py |  4 +-
 features/steps/core.py  | 11 ++----
 jrnl/color.py           |  2 +-
 jrnl/editor.py          |  6 +--
 jrnl/os_compat.py       | 14 ++++++-
 tests/test_os_compat.py | 87 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 109 insertions(+), 15 deletions(-)
 create mode 100644 tests/test_os_compat.py

diff --git a/features/environment.py b/features/environment.py
index 71ed8969..f4baab34 100644
--- a/features/environment.py
+++ b/features/environment.py
@@ -42,7 +42,7 @@ def before_feature(context, feature):
         feature.skip()
         return
 
-    if "skip_win" in feature.tags and on_windows:
+    if "skip_win" in feature.tags and on_windows():
         feature.skip("Skipping on Windows")
         return
 
@@ -69,7 +69,7 @@ def before_scenario(context, scenario):
         scenario.skip()
         return
 
-    if "skip_win" in scenario.effective_tags and on_windows:
+    if "skip_win" in scenario.effective_tags and on_windows():
         scenario.skip("Skipping on Windows")
         return
 
diff --git a/features/steps/core.py b/features/steps/core.py
index e3af8243..46b9ec47 100644
--- a/features/steps/core.py
+++ b/features/steps/core.py
@@ -6,7 +6,6 @@ from collections import defaultdict
 import os
 from pathlib import Path
 import re
-import shlex
 import time
 from unittest.mock import patch
 
@@ -22,7 +21,7 @@ from jrnl import __version__
 from jrnl import plugins
 from jrnl.cli import cli
 from jrnl.config import load_config
-from jrnl.os_compat import on_windows
+from jrnl.os_compat import split_args
 
 try:
     import parsedatetime.parsedatetime_consts as pdt
@@ -87,10 +86,6 @@ class FailedKeyring(keyring.backend.KeyringBackend):
 keyring.set_keyring(TestKeyring())
 
 
-def ushlex(command):
-    return shlex.split(command, posix=not on_windows)
-
-
 def read_journal(context, journal_name="default"):
     configuration = load_config(context.config_path)
     with open(configuration["journals"][journal_name]) as journal_file:
@@ -300,7 +295,7 @@ def run_with_input(context, command, inputs=""):
     else:
         text = iter([inputs])
 
-    args = ushlex(command)[1:]
+    args = split_args(command)[1:]
 
     def _mock_editor(command):
         context.editor_command = command
@@ -383,7 +378,7 @@ def run(context, command, text=""):
         cache_dir = os.path.join("features", "cache", context.cache_dir)
         command = command.format(cache_dir=cache_dir)
 
-    args = ushlex(command)
+    args = split_args(command)
 
     def _mock_editor(command):
         context.editor_command = command
diff --git a/jrnl/color.py b/jrnl/color.py
index dca28117..3bdd4149 100644
--- a/jrnl/color.py
+++ b/jrnl/color.py
@@ -7,7 +7,7 @@ import colorama
 
 from .os_compat import on_windows
 
-if on_windows:
+if on_windows():
     colorama.init()
 
 WARNING_COLOR = colorama.Fore.YELLOW
diff --git a/jrnl/editor.py b/jrnl/editor.py
index 1a68028d..086d84db 100644
--- a/jrnl/editor.py
+++ b/jrnl/editor.py
@@ -1,6 +1,5 @@
 import logging
 import os
-import shlex
 import subprocess
 import sys
 import tempfile
@@ -10,6 +9,7 @@ from pathlib import Path
 from .color import ERROR_COLOR
 from .color import RESET_COLOR
 from .os_compat import on_windows
+from .os_compat import split_args
 
 
 def get_text_from_editor(config, template=""):
@@ -25,7 +25,7 @@ def get_text_from_editor(config, template=""):
             f.write(template)
 
     try:
-        subprocess.call(shlex.split(config["editor"], posix=on_windows) + [tmpfile])
+        subprocess.call(split_args(config["editor"]) + [tmpfile])
     except Exception as e:
         error_msg = f"""
         {ERROR_COLOR}{str(e)}{RESET_COLOR}
@@ -47,7 +47,7 @@ def get_text_from_editor(config, template=""):
 
 
 def get_text_from_stdin():
-    _how_to_quit = "Ctrl+z and then Enter" if on_windows else "Ctrl+d"
+    _how_to_quit = "Ctrl+z and then Enter" if on_windows() else "Ctrl+d"
     print(
         f"[Writing Entry; on a blank line, press {_how_to_quit} to finish writing]\n",
         file=sys.stderr,
diff --git a/jrnl/os_compat.py b/jrnl/os_compat.py
index b38d9d60..2da13515 100644
--- a/jrnl/os_compat.py
+++ b/jrnl/os_compat.py
@@ -1,6 +1,18 @@
 # Copyright (C) 2012-2021 jrnl contributors
 # License: https://www.gnu.org/licenses/gpl-3.0.html
 
+import shlex
 from sys import platform
 
-on_windows = "win32" in platform
+
+def on_windows():
+    return "win32" in platform
+
+
+def on_posix():
+    return not on_windows()
+
+
+def split_args(args):
+    """Split arguments and add escape characters as appropriate for the OS"""
+    return shlex.split(args, posix=on_posix)
diff --git a/tests/test_os_compat.py b/tests/test_os_compat.py
new file mode 100644
index 00000000..9b31de50
--- /dev/null
+++ b/tests/test_os_compat.py
@@ -0,0 +1,87 @@
+from unittest import mock
+import pytest
+
+from jrnl.os_compat import on_windows
+from jrnl.os_compat import on_posix
+from jrnl.os_compat import split_args
+
+
+@pytest.mark.parametrize(
+    "systems",
+    [
+        ["linux", False],
+        ["win32", True],
+        ["cygwin", False],
+        ["msys", False],
+        ["darwin", False],
+        ["os2", False],
+        ["os2emx", False],
+        ["riscos", False],
+        ["atheos", False],
+        ["freebsd7", False],
+        ["freebsd8", False],
+        ["freebsdN", False],
+        ["openbsd6", False],
+    ],
+)
+def test_on_windows(systems):
+    osname, expected_on_windows = systems[0], systems[1]
+    with mock.patch("jrnl.os_compat.platform", osname):
+        assert on_windows() == expected_on_windows
+
+
+@pytest.mark.parametrize(
+    "systems",
+    [
+        ["linux", True],
+        ["win32", False],
+        ["cygwin", True],
+        ["msys", True],
+        ["darwin", True],
+        ["os2", True],
+        ["os2emx", True],
+        ["riscos", True],
+        ["atheos", True],
+        ["freebsd7", True],
+        ["freebsd8", True],
+        ["freebsdN", True],
+        ["openbsd6", True],
+    ],
+)
+def test_on_posix(systems):
+    osname, expected_on_posix = systems[0], systems[1]
+    with mock.patch("jrnl.os_compat.platform", osname):
+        assert on_posix() == expected_on_posix
+
+
+@pytest.mark.parametrize(
+    "args",
+    [
+        ["notepad", ["notepad"]],
+        ["subl -w", ["subl", "-w"]],
+        [
+            '"C:\\Program Files\\Sublime Text 3\\subl.exe" -w',
+            ["C:\\Program Files\\Sublime Text 3\\subl.exe", "-w"],
+        ],
+    ],
+)
+def test_split_args_on_windows(args):
+    input_arguments, expected_split_args = args[0], args[1]
+    with mock.patch("jrnl.os_compat.on_windows", True):
+        assert split_args(input_arguments) == expected_split_args
+
+
+@pytest.mark.parametrize(
+    "args",
+    [
+        ["vim", ["vim"]],
+        [
+            'vim -f +Goyo +Limelight "+set spell linebreak"',
+            ["vim", "-f", "+Goyo", "+Limelight", "+set spell linebreak"],
+        ],
+    ],
+)
+def test_split_args_on_not_windows(args):
+    input_arguments, expected_split_args = args[0], args[1]
+    with mock.patch("jrnl.os_compat.on_windows", True):
+        assert split_args(input_arguments) == expected_split_args