Notify user when config directory can't be created because there is already a file with the same name (#1134)

* Moving configuration values and methods from install.py to config.py -- everything is broken right now
* Using context to store config path - still lots broken
* Use mocks and context.config_path to store test configs - many tests still broken though
* Update changelog [ci skip]
* Fix jrnl --ls crash
* Fix crash when no editor configured
* Attempt to patch config path with test data - doesn't appear to be working
* Properly use patched config path and add config given to scenario that wasn't using it
* Fix copypasta
* Fix editor test that needed patched config and trapping for system exit
* Add exception and handling for when configuration directory is actually a file
* Remove extraneous comment
* Use more generic JrnlError with messaging switchboard
* Format code a bit nicer
* Remove unnecessary given in diagnostic test
* Ensure full error message is output
* Remove unnecessary whitespace characters

features/steps/core.py

@@ -19,7 +19,6 @@ import yaml
 
 from jrnl import Journal
 from jrnl import __version__
-from jrnl import install
 from jrnl import plugins
 from jrnl.cli import cli
 from jrnl.config import load_config
@@ -92,25 +91,25 @@ def ushlex(command):
     return shlex.split(command, posix=not on_windows)
 
 
-def read_journal(journal_name="default"):
+def read_journal(context, journal_name="default"):
-    config = load_config(install.CONFIG_FILE_PATH)
+    configuration = load_config(context.config_path)
-    with open(config["journals"][journal_name]) as journal_file:
+    with open(configuration["journals"][journal_name]) as journal_file:
         journal = journal_file.read()
     return journal
 
 
-def open_journal(journal_name="default"):
+def open_journal(context, journal_name="default"):
-    config = load_config(install.CONFIG_FILE_PATH)
+    configuration = load_config(context.config_path)
-    journal_conf = config["journals"][journal_name]
+    journal_conf = configuration["journals"][journal_name]
 
     # We can override the default config on a by-journal basis
     if type(journal_conf) is dict:
-        config.update(journal_conf)
+        configuration.update(journal_conf)
     # But also just give them a string to point to the journal file
     else:
-        config["journal"] = journal_conf
+        configuration["journal"] = journal_conf
 
-    return Journal.open_journal(journal_name, config)
+    return Journal.open_journal(journal_name, configuration)
 
 
 def read_value_from_string(string):
@@ -128,10 +127,11 @@ def read_value_from_string(string):
 def set_config(context, config_file):
     full_path = os.path.join("features/configs", config_file)
 
-    install.CONFIG_FILE_PATH = os.path.abspath(full_path)
+    context.config_path = os.path.abspath(full_path)
+
     if config_file.endswith("yaml") and os.path.exists(full_path):
         # Add jrnl version to file for 2.x journals
-        with open(install.CONFIG_FILE_PATH, "a") as cf:
+        with open(context.config_path, "a") as cf:
             cf.write("version: {}".format(__version__))
 
 
@@ -196,11 +196,18 @@ def open_editor_and_enter(context, method, text=""):
     with \
         patch("subprocess.call", side_effect=_mock_editor) as mock_editor, \
         patch("getpass.getpass", side_effect=_mock_getpass(password)) as mock_getpass, \
-        patch("sys.stdin.isatty", return_value=True) \
+        patch("sys.stdin.isatty", return_value=True), \
+        patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \
+        patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \
     :
         context.editor = mock_editor
         context.getpass = mock_getpass
-        cli(["--edit"])
+        try:
+            cli(["--edit"])
+            context.exit_status = 0
+        except SystemExit as e:
+            context.exit_status = e.code
+
     # fmt: on
 
 
@@ -314,7 +321,9 @@ def run_with_input(context, command, inputs=""):
         patch("builtins.input", side_effect=_mock_input(text)) as mock_input, \
         patch("getpass.getpass", side_effect=_mock_getpass(password)) as mock_getpass, \
         patch("sys.stdin.read", side_effect=text) as mock_read, \
-        patch("subprocess.call", side_effect=_mock_editor) as mock_editor \
+        patch("subprocess.call", side_effect=_mock_editor) as mock_editor, \
+        patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \
+        patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \
     :
         try:
             cli(args or [])
@@ -396,7 +405,9 @@ def run(context, command, text=""):
             patch("sys.argv", args), \
             patch("getpass.getpass", side_effect=_mock_getpass(password)) as mock_getpass, \
             patch("subprocess.call", side_effect=_mock_editor) as mock_editor, \
-            patch("sys.stdin.read", side_effect=lambda: text) \
+            patch("sys.stdin.read", side_effect=lambda: text), \
+            patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \
+            patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \
         :
             context.editor = mock_editor
             context.getpass = mock_getpass
@@ -544,32 +555,32 @@ def check_not_message(context, text):
 @then('the journal should contain "{text}"')
 @then('journal "{journal_name}" should contain "{text}"')
 def check_journal_content(context, text, journal_name="default"):
-    journal = read_journal(journal_name)
+    journal = read_journal(context, journal_name)
     assert text in journal, journal
 
 
 @then('the journal should not contain "{text}"')
 @then('journal "{journal_name}" should not contain "{text}"')
 def check_not_journal_content(context, text, journal_name="default"):
-    journal = read_journal(journal_name)
+    journal = read_journal(context, journal_name)
     assert text not in journal, journal
 
 
 @then("the journal should not exist")
 @then('journal "{journal_name}" should not exist')
 def journal_doesnt_exist(context, journal_name="default"):
-    config = load_config(install.CONFIG_FILE_PATH)
+    configuration = load_config(context.config_path)
 
-    journal_path = config["journals"][journal_name]
+    journal_path = configuration["journals"][journal_name]
     assert not os.path.exists(journal_path)
 
 
 @then("the journal should exist")
 @then('journal "{journal_name}" should exist')
 def journal_exists(context, journal_name="default"):
-    config = load_config(install.CONFIG_FILE_PATH)
+    configuration = load_config(context.config_path)
 
-    journal_path = config["journals"][journal_name]
+    journal_path = configuration["journals"][journal_name]
     assert os.path.exists(journal_path)
 
 
@@ -578,23 +589,23 @@ def journal_exists(context, journal_name="default"):
 @then('the config for journal "{journal}" should have "{key}" set to "{value}"')
 def config_var(context, key, value="", journal=None):
     value = read_value_from_string(value or context.text or "")
-    config = load_config(install.CONFIG_FILE_PATH)
+    configuration = load_config(context.config_path)
 
     if journal:
-        config = config["journals"][journal]
+        configuration = configuration["journals"][journal]
 
-    assert key in config
+    assert key in configuration
-    assert config[key] == value
+    assert configuration[key] == value
 
 
 @then('the config for journal "{journal}" should not have "{key}" set')
 def config_no_var(context, key, value="", journal=None):
-    config = load_config(install.CONFIG_FILE_PATH)
+    configuration = load_config(context.config_path)
 
     if journal:
-        config = config["journals"][journal]
+        configuration = configuration["journals"][journal]
 
-    assert key not in config
+    assert key not in configuration
 
 
 @then("the journal should have {number:d} entries")
@@ -602,15 +613,15 @@ def config_no_var(context, key, value="", journal=None):
 @then('journal "{journal_name}" should have {number:d} entries')
 @then('journal "{journal_name}" should have {number:d} entry')
 def check_journal_entries(context, number, journal_name="default"):
-    journal = open_journal(journal_name)
+    journal = open_journal(context, journal_name)
     assert len(journal.entries) == number
 
 
 @when("the journal directory is listed")
 def list_journal_directory(context, journal="default"):
-    with open(install.CONFIG_FILE_PATH) as config_file:
+    with open(context.config_path) as config_file:
-        config = yaml.load(config_file, Loader=yaml.FullLoader)
+        configuration = yaml.load(config_file, Loader=yaml.FullLoader)
-    journal_path = config["journals"][journal]
+    journal_path = configuration["journals"][journal]
     for root, dirnames, f in os.walk(journal_path):
         for file in f:
             print(os.path.join(root, file))

jrnl/cli.py

@@ -7,6 +7,7 @@ import sys
 
 from .jrnl import run
 from .args import parse_args
+from .exception import JrnlError
 
 
 def configure_logger(debug=False):
@@ -33,5 +34,9 @@ def cli(manual_args=None):
 
         return run(args)
 
+    except JrnlError as e:
+        print(e.message, file=sys.stderr)
+        return 1
+
     except KeyboardInterrupt:
         return 1

jrnl/config.py

@@ -1,13 +1,77 @@
 import logging
+import os
 import sys
 
 import colorama
 import yaml
+import xdg.BaseDirectory
 
+from . import __version__
+from .exception import JrnlError
 from .color import ERROR_COLOR
 from .color import RESET_COLOR
 from .output import list_journals
 
+# Constants
+DEFAULT_CONFIG_NAME = "jrnl.yaml"
+XDG_RESOURCE = "jrnl"
+
+DEFAULT_JOURNAL_NAME = "journal.txt"
+DEFAULT_JOURNAL_KEY = "default"
+
+
+def save_config(config):
+    config["version"] = __version__
+    with open(get_config_path(), "w") as f:
+        yaml.safe_dump(
+            config, f, encoding="utf-8", allow_unicode=True, default_flow_style=False
+        )
+
+
+def get_config_path():
+    try:
+        config_directory_path = xdg.BaseDirectory.save_config_path(XDG_RESOURCE)
+    except FileExistsError:
+        raise JrnlError(
+            "ConfigDirectoryIsFile",
+            config_directory_path=os.path.join(
+                xdg.BaseDirectory.xdg_config_home, XDG_RESOURCE
+            ),
+        )
+    return os.path.join(
+        config_directory_path or os.path.expanduser("~"), DEFAULT_CONFIG_NAME
+    )
+
+
+def get_default_config():
+    return {
+        "version": __version__,
+        "journals": {"default": get_default_journal_path()},
+        "editor": os.getenv("VISUAL") or os.getenv("EDITOR") or "",
+        "encrypt": False,
+        "template": False,
+        "default_hour": 9,
+        "default_minute": 0,
+        "timeformat": "%Y-%m-%d %H:%M",
+        "tagsymbols": "@",
+        "highlight": True,
+        "linewrap": 79,
+        "indent_character": "|",
+        "colors": {
+            "date": "none",
+            "title": "none",
+            "body": "none",
+            "tags": "none",
+        },
+    }
+
+
+def get_default_journal_path():
+    journal_data_path = xdg.BaseDirectory.save_data_path(
+        XDG_RESOURCE
+    ) or os.path.expanduser("~")
+    return os.path.join(journal_data_path, DEFAULT_JOURNAL_NAME)
+
 
 def scope_config(config, journal_name):
     if journal_name not in config["journals"]:
@@ -73,13 +137,11 @@ def update_config(config, new_config, scope, force_local=False):
 
 
 def get_journal_name(args, config):
-    from . import install
+    args.journal_name = DEFAULT_JOURNAL_KEY
-
-    args.journal_name = install.DEFAULT_JOURNAL_KEY
     if args.text and args.text[0] in config["journals"]:
         args.journal_name = args.text[0]
         args.text = args.text[1:]
-    elif install.DEFAULT_JOURNAL_KEY not in config["journals"]:
+    elif DEFAULT_JOURNAL_KEY not in config["journals"]:
         print("No default journal configured.", file=sys.stderr)
         print(list_journals(config), file=sys.stderr)
         sys.exit(1)

jrnl/exception.py

@@ -1,5 +1,6 @@
 # Copyright (C) 2012-2021 jrnl contributors
 # License: https://www.gnu.org/licenses/gpl-3.0.html
+import textwrap
 
 
 class UserAbort(Exception):
@@ -10,3 +11,28 @@ class UpgradeValidationException(Exception):
     """Raised when the contents of an upgraded journal do not match the old journal"""
 
     pass
+
+
+class JrnlError(Exception):
+    """Common exceptions raised by jrnl. """
+
+    def __init__(self, error_type, **kwargs):
+        self.error_type = error_type
+        self.message = self._get_error_message(**kwargs)
+
+    def _get_error_message(self, **kwargs):
+        error_messages = {
+            "ConfigDirectoryIsFile": textwrap.dedent(
+                """
+                The path to your jrnl configuration directory is a file, not a directory:
+
+                {config_directory_path}
+
+                Removing this file will allow jrnl to save its configuration.
+            """
+            )
+        }
+
+        return error_messages[self.error_type].format(**kwargs)
+
+    pass

jrnl/install.py

@@ -8,86 +8,45 @@ import logging
 import os
 import sys
 
-import xdg.BaseDirectory
+from .config import DEFAULT_JOURNAL_KEY
-import yaml
+from .config import get_config_path
-
+from .config import get_default_config
-from . import __version__
+from .config import get_default_journal_path
 from .config import load_config
+from .config import save_config
 from .config import verify_config_colors
 from .exception import UserAbort
 from .prompt import yesno
 from .upgrade import is_old_version
 
-DEFAULT_CONFIG_NAME = "jrnl.yaml"
-DEFAULT_JOURNAL_NAME = "journal.txt"
-DEFAULT_JOURNAL_KEY = "default"
-XDG_RESOURCE = "jrnl"
-
-USER_HOME = os.path.expanduser("~")
-
-CONFIG_PATH = xdg.BaseDirectory.save_config_path(XDG_RESOURCE) or USER_HOME
-CONFIG_FILE_PATH = os.path.join(CONFIG_PATH, DEFAULT_CONFIG_NAME)
-CONFIG_FILE_PATH_FALLBACK = os.path.join(USER_HOME, ".jrnl_config")
-
-JOURNAL_PATH = xdg.BaseDirectory.save_data_path(XDG_RESOURCE) or USER_HOME
-JOURNAL_FILE_PATH = os.path.join(JOURNAL_PATH, DEFAULT_JOURNAL_NAME)
-
-
-default_config = {
-    "version": __version__,
-    "journals": {"default": JOURNAL_FILE_PATH},
-    "editor": os.getenv("VISUAL") or os.getenv("EDITOR") or "",
-    "encrypt": False,
-    "template": False,
-    "default_hour": 9,
-    "default_minute": 0,
-    "timeformat": "%Y-%m-%d %H:%M",
-    "tagsymbols": "@",
-    "highlight": True,
-    "linewrap": 79,
-    "indent_character": "|",
-    "colors": {
-        "date": "none",
-        "title": "none",
-        "body": "none",
-        "tags": "none",
-    },
-}
-
 
 def upgrade_config(config):
     """Checks if there are keys missing in a given config dict, and if so, updates the config file accordingly.
     This essentially automatically ports jrnl installations if new config parameters are introduced in later
     versions."""
+    default_config = get_default_config()
     missing_keys = set(default_config).difference(config)
     if missing_keys:
         for key in missing_keys:
             config[key] = default_config[key]
         save_config(config)
         print(
-            f"[Configuration updated to newest version at {CONFIG_FILE_PATH}]",
+            f"[Configuration updated to newest version at {get_config_path()}]",
             file=sys.stderr,
         )
 
 
-def save_config(config):
-    config["version"] = __version__
-    with open(CONFIG_FILE_PATH, "w") as f:
-        yaml.safe_dump(
-            config, f, encoding="utf-8", allow_unicode=True, default_flow_style=False
-        )
-
-
 def load_or_install_jrnl():
     """
     If jrnl is already installed, loads and returns a config object.
     Else, perform various prompts to install jrnl.
     """
     config_path = (
-        CONFIG_FILE_PATH
+        get_config_path()
-        if os.path.exists(CONFIG_FILE_PATH)
+        if os.path.exists(get_config_path())
-        else CONFIG_FILE_PATH_FALLBACK
+        else os.path.join(os.path.expanduser("~"), ".jrnl_config")
     )
+
     if os.path.exists(config_path):
         logging.debug("Reading configuration from file %s", config_path)
         config = load_config(config_path)
@@ -128,8 +87,10 @@ def install():
     _initialize_autocomplete()
 
     # Where to create the journal?
-    path_query = f"Path to your journal file (leave blank for {JOURNAL_FILE_PATH}): "
+    default_journal_path = get_default_journal_path()
-    journal_path = os.path.abspath(input(path_query).strip() or JOURNAL_FILE_PATH)
+    path_query = f"Path to your journal file (leave blank for {default_journal_path}): "
+    journal_path = os.path.abspath(input(path_query).strip() or default_journal_path)
+    default_config = get_default_config()
     default_config["journals"][DEFAULT_JOURNAL_KEY] = os.path.expanduser(
         os.path.expandvars(journal_path)
     )

jrnl/jrnl.py

@@ -11,6 +11,7 @@ from .color import ERROR_COLOR
 from .color import RESET_COLOR
 from .config import get_journal_name
 from .config import scope_config
+from .config import get_config_path
 from .editor import get_text_from_editor
 from .editor import get_text_from_stdin
 from .exception import UserAbort
@@ -228,7 +229,7 @@ def _edit_search_results(config, journal, old_entries, **kwargs):
             f"""
             [{ERROR_COLOR}ERROR{RESET_COLOR}: There is no editor configured.]
 
-            Please specify an editor in config file ({install.CONFIG_FILE_PATH})
+            Please specify an editor in config file ({get_config_path()})
             to use the --edit option.
             """,
             file=sys.stderr,

jrnl/output.py

@@ -23,13 +23,13 @@ def deprecated_cmd(old_cmd, new_cmd, callback=None, **kwargs):
         callback(**kwargs)
 
 
-def list_journals(config):
+def list_journals(configuration):
-    from . import install
+    from . import config
 
     """List the journals specified in the configuration file"""
-    result = f"Journals defined in {install.CONFIG_FILE_PATH}\n"
+    result = f"Journals defined in {config.get_config_path()}\n"
-    ml = min(max(len(k) for k in config["journals"]), 20)
+    ml = min(max(len(k) for k in configuration["journals"]), 20)
-    for journal, cfg in config["journals"].items():
+    for journal, cfg in configuration["journals"].items():
         result += " * {:{}} -> {}\n".format(
             journal, ml, cfg["journal"] if isinstance(cfg, dict) else cfg
         )

tests/test_exception.py

@@ -0,0 +1,19 @@
+import textwrap
+
+from jrnl.exception import JrnlError
+
+
+def test_config_directory_exception_message():
+    ex = JrnlError(
+        "ConfigDirectoryIsFile", config_directory_path="/config/directory/path"
+    )
+
+    assert ex.message == textwrap.dedent(
+        """
+        The path to your jrnl configuration directory is a file, not a directory:
+        
+        /config/directory/path
+        
+        Removing this file will allow jrnl to save its configuration.
+		"""
+    )