From 63850a33c12da1fc623721ef4fd2e2faa598ef88 Mon Sep 17 00:00:00 2001 From: Jonathan Wren Date: Sat, 22 Oct 2022 15:35:16 -0700 Subject: [PATCH] Fix bug for new `--list --format` options when no default journal is specified (#1621) * rename test config * Change journal name validation Journal name validation used to happen before postconfig commands could have a chance to run, so now each command is responsible for its own error-checking of the journal name. Added a new decorator and function that makes this error-checking easier. Co-authored-by: Micah Jerome Ellison * fix wrapper function call to be more like original * change arg names to show which aren't used * add type hints Co-authored-by: Micah Jerome Ellison --- jrnl/Journal.py | 2 ++ jrnl/commands.py | 25 +++++++++++--- jrnl/config.py | 33 ++++++++++++++----- jrnl/messages/MsgText.py | 2 +- tests/bdd/features/config_file.feature | 6 ++-- tests/bdd/features/format.feature | 17 ++++++++++ tests/bdd/features/multiple_journals.feature | 4 +-- .../{bug343.yaml => no_default_journal.yaml} | 0 8 files changed, 70 insertions(+), 19 deletions(-) rename tests/data/configs/{bug343.yaml => no_default_journal.yaml} (100%) diff --git a/jrnl/Journal.py b/jrnl/Journal.py index c2c43142..2fa1d465 100644 --- a/jrnl/Journal.py +++ b/jrnl/Journal.py @@ -8,6 +8,7 @@ import re from jrnl import Entry from jrnl import time +from jrnl.config import validate_journal_name from jrnl.messages import Message from jrnl.messages import MsgStyle from jrnl.messages import MsgText @@ -430,6 +431,7 @@ def open_journal(journal_name, config, legacy=False): If legacy is True, it will open Journals with legacy classes build for backwards compatibility with jrnl 1.x """ + validate_journal_name(journal_name, config) config = config.copy() config["journal"] = expand_path(config["journal"]) diff --git a/jrnl/commands.py b/jrnl/commands.py index 6100422b..b1fc81e0 100644 --- a/jrnl/commands.py +++ b/jrnl/commands.py @@ -14,9 +14,11 @@ run. Also, please note that all (non-builtin) imports should be scoped to each function to avoid any possible overhead for these standalone commands. """ +import argparse import platform import sys +from jrnl.config import cmd_requires_valid_journal_name from jrnl.exception import JrnlException from jrnl.messages import Message from jrnl.messages import MsgStyle @@ -56,13 +58,16 @@ def preconfig_version(_): print(output) -def postconfig_list(args, config, **kwargs): +def postconfig_list(args: argparse.Namespace, config: dict, **_) -> int: from jrnl.output import list_journals print(list_journals(config, args.export)) + return 0 -def postconfig_import(args, config, **kwargs): + +@cmd_requires_valid_journal_name +def postconfig_import(args: argparse.Namespace, config: dict, **_) -> int: from jrnl.Journal import open_journal from jrnl.plugins import get_importer @@ -72,8 +77,13 @@ def postconfig_import(args, config, **kwargs): format = args.export if args.export else "jrnl" get_importer(format).import_(journal, args.filename) + return 0 -def postconfig_encrypt(args, config, original_config, **kwargs): + +@cmd_requires_valid_journal_name +def postconfig_encrypt( + args: argparse.Namespace, config: dict, original_config: dict +) -> int: """ Encrypt a journal in place, or optionally to a new file """ @@ -122,8 +132,13 @@ def postconfig_encrypt(args, config, original_config, **kwargs): ) save_config(original_config) + return 0 -def postconfig_decrypt(args, config, original_config, **kwargs): + +@cmd_requires_valid_journal_name +def postconfig_decrypt( + args: argparse.Namespace, config: dict, original_config: dict +) -> int: """Decrypts into new file. If filename is not set, we encrypt the journal file itself.""" from jrnl.config import update_config from jrnl.install import save_config @@ -149,3 +164,5 @@ def postconfig_decrypt(args, config, original_config, **kwargs): original_config, {"encrypt": False}, args.journal_name, force_local=True ) save_config(original_config) + + return 0 diff --git a/jrnl/config.py b/jrnl/config.py index aed17200..8e5c5a14 100644 --- a/jrnl/config.py +++ b/jrnl/config.py @@ -1,8 +1,10 @@ # Copyright © 2012-2022 jrnl contributors # License: https://www.gnu.org/licenses/gpl-3.0.html +import argparse import logging import os +from typing import Callable import colorama import xdg.BaseDirectory @@ -213,14 +215,27 @@ def get_journal_name(args, config): args.journal_name = potential_journal_name args.text = args.text[1:] - if args.journal_name not in config["journals"]: - raise JrnlException( - Message( - MsgText.NoDefaultJournal, - MsgStyle.ERROR, - {"journals": list_journals(config)}, - ), - ) - logging.debug("Using journal name: %s", args.journal_name) return args + + +def cmd_requires_valid_journal_name(func: Callable) -> Callable: + def wrapper(args: argparse.Namespace, config: dict, original_config: dict): + validate_journal_name(args.journal_name, config) + func(args=args, config=config, original_config=original_config) + + return wrapper + + +def validate_journal_name(journal_name: str, config: dict) -> None: + if journal_name not in config["journals"]: + raise JrnlException( + Message( + MsgText.NoNamedJournal, + MsgStyle.ERROR, + { + "journal_name": journal_name, + "journals": list_journals(config), + }, + ), + ) diff --git a/jrnl/messages/MsgText.py b/jrnl/messages/MsgText.py index 0f509645..b3cc50e7 100644 --- a/jrnl/messages/MsgText.py +++ b/jrnl/messages/MsgText.py @@ -101,7 +101,7 @@ class MsgText(Enum): {template} """ - NoDefaultJournal = "No default journal configured\n{journals}" + NoNamedJournal = "No '{journal_name}' journal configured\n{journals}" DoesNotExist = "{name} does not exist" diff --git a/tests/bdd/features/config_file.feature b/tests/bdd/features/config_file.feature index 6c24f1e5..3369e666 100644 --- a/tests/bdd/features/config_file.feature +++ b/tests/bdd/features/config_file.feature @@ -64,10 +64,10 @@ Feature: Multiple journals Then the output should contain "sell my junk on ebay and make lots of money" Scenario: Don't crash if no default journal is specified using an alternate config - Given the config "bug343.yaml" exists + Given the config "no_default_journal.yaml" exists And we use the config "basic_onefile.yaml" - When we run "jrnl --cf bug343.yaml a long day in the office" - Then the output should contain "No default journal configured" + When we run "jrnl --cf no_default_journal.yaml a long day in the office" + Then the output should contain "No 'default' journal configured" Scenario: Don't crash if no file exists for a configured encrypted journal using an alternate config Given the config "multiple.yaml" exists diff --git a/tests/bdd/features/format.feature b/tests/bdd/features/format.feature index 2cc7d9a1..54715495 100644 --- a/tests/bdd/features/format.feature +++ b/tests/bdd/features/format.feature @@ -612,3 +612,20 @@ Feature: Custom formats config_path: .+basic_onefile\.yaml journals: default: features/journals/basic_onefile\.journal + + Scenario: Export journal list to formats with no default journal + Given we use the config "no_default_journal.yaml" + When we run "jrnl --list" + Then the output should match + Journals defined in config \(.+no_default_journal\.yaml\) + \* simple -> features/journals/simple\.journal + \* work -> features/journals/work\.journal + When we run "jrnl --list --format json" + Then the output should match + {"config_path": ".+no_default_journal\.yaml", "journals": {"simple": "features/journals/simple\.journal", "work": "features/journals/work\.journal"}} + When we run "jrnl --list --format yaml" + Then the output should match + config_path: .+no_default_journal\.yaml + journals: + simple: features/journals/simple\.journal + work: features/journals/work\.journal diff --git a/tests/bdd/features/multiple_journals.feature b/tests/bdd/features/multiple_journals.feature index 3c2c7b73..ee90b8a9 100644 --- a/tests/bdd/features/multiple_journals.feature +++ b/tests/bdd/features/multiple_journals.feature @@ -80,9 +80,9 @@ Feature: Multiple journals 2012-07-23 09:00 sell my junk on ebay and make lots of money Scenario: Don't crash if no default journal is specified - Given we use the config "bug343.yaml" + Given we use the config "no_default_journal.yaml" When we run "jrnl a long day in the office" - Then the output should contain "No default journal configured" + Then the output should contain "No 'default' journal configured" Scenario: Don't crash if no file exists for a configured encrypted journal Given we use the config "multiple.yaml" diff --git a/tests/data/configs/bug343.yaml b/tests/data/configs/no_default_journal.yaml similarity index 100% rename from tests/data/configs/bug343.yaml rename to tests/data/configs/no_default_journal.yaml