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 <micah.jerome.ellison@gmail.com>

* 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 <micah.jerome.ellison@gmail.com>
This commit is contained in:
Jonathan Wren 2022-10-22 15:35:16 -07:00 committed by GitHub
parent 619de775fd
commit 63850a33c1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 70 additions and 19 deletions

View file

@ -8,6 +8,7 @@ import re
from jrnl import Entry from jrnl import Entry
from jrnl import time from jrnl import time
from jrnl.config import validate_journal_name
from jrnl.messages import Message from jrnl.messages import Message
from jrnl.messages import MsgStyle from jrnl.messages import MsgStyle
from jrnl.messages import MsgText 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 If legacy is True, it will open Journals with legacy classes build for
backwards compatibility with jrnl 1.x backwards compatibility with jrnl 1.x
""" """
validate_journal_name(journal_name, config)
config = config.copy() config = config.copy()
config["journal"] = expand_path(config["journal"]) config["journal"] = expand_path(config["journal"])

View file

@ -14,9 +14,11 @@ run.
Also, please note that all (non-builtin) imports should be scoped to each function to Also, please note that all (non-builtin) imports should be scoped to each function to
avoid any possible overhead for these standalone commands. avoid any possible overhead for these standalone commands.
""" """
import argparse
import platform import platform
import sys import sys
from jrnl.config import cmd_requires_valid_journal_name
from jrnl.exception import JrnlException from jrnl.exception import JrnlException
from jrnl.messages import Message from jrnl.messages import Message
from jrnl.messages import MsgStyle from jrnl.messages import MsgStyle
@ -56,13 +58,16 @@ def preconfig_version(_):
print(output) print(output)
def postconfig_list(args, config, **kwargs): def postconfig_list(args: argparse.Namespace, config: dict, **_) -> int:
from jrnl.output import list_journals from jrnl.output import list_journals
print(list_journals(config, args.export)) 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.Journal import open_journal
from jrnl.plugins import get_importer from jrnl.plugins import get_importer
@ -72,8 +77,13 @@ def postconfig_import(args, config, **kwargs):
format = args.export if args.export else "jrnl" format = args.export if args.export else "jrnl"
get_importer(format).import_(journal, args.filename) 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 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) 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.""" """Decrypts into new file. If filename is not set, we encrypt the journal file itself."""
from jrnl.config import update_config from jrnl.config import update_config
from jrnl.install import save_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 original_config, {"encrypt": False}, args.journal_name, force_local=True
) )
save_config(original_config) save_config(original_config)
return 0

View file

@ -1,8 +1,10 @@
# Copyright © 2012-2022 jrnl contributors # Copyright © 2012-2022 jrnl contributors
# License: https://www.gnu.org/licenses/gpl-3.0.html # License: https://www.gnu.org/licenses/gpl-3.0.html
import argparse
import logging import logging
import os import os
from typing import Callable
import colorama import colorama
import xdg.BaseDirectory import xdg.BaseDirectory
@ -213,14 +215,27 @@ def get_journal_name(args, config):
args.journal_name = potential_journal_name args.journal_name = potential_journal_name
args.text = args.text[1:] 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) logging.debug("Using journal name: %s", args.journal_name)
return args 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),
},
),
)

View file

@ -101,7 +101,7 @@ class MsgText(Enum):
{template} {template}
""" """
NoDefaultJournal = "No default journal configured\n{journals}" NoNamedJournal = "No '{journal_name}' journal configured\n{journals}"
DoesNotExist = "{name} does not exist" DoesNotExist = "{name} does not exist"

View file

@ -64,10 +64,10 @@ Feature: Multiple journals
Then the output should contain "sell my junk on ebay and make lots of money" 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 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" And we use the config "basic_onefile.yaml"
When we run "jrnl --cf bug343.yaml a long day in the office" When we run "jrnl --cf no_default_journal.yaml 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 using an alternate config Scenario: Don't crash if no file exists for a configured encrypted journal using an alternate config
Given the config "multiple.yaml" exists Given the config "multiple.yaml" exists

View file

@ -612,3 +612,20 @@ Feature: Custom formats
config_path: .+basic_onefile\.yaml config_path: .+basic_onefile\.yaml
journals: journals:
default: features/journals/basic_onefile\.journal 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

View file

@ -80,9 +80,9 @@ Feature: Multiple journals
2012-07-23 09:00 sell my junk on ebay and make lots of money 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 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" 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 Scenario: Don't crash if no file exists for a configured encrypted journal
Given we use the config "multiple.yaml" Given we use the config "multiple.yaml"