Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--format list and error output is incorrect for importing #1486

Open
micahellison opened this issue May 23, 2022 · 0 comments · May be fixed by #1959
Open

--format list and error output is incorrect for importing #1486

micahellison opened this issue May 23, 2022 · 0 comments · May be fixed by #1959
Labels
bug Something isn't working ready for pr Okay to start work. Feel free to ask questions.
Milestone

Comments

@micahellison
Copy link
Member

Bug Report

Environment

  • jrnl --diagnostic output:
jrnl: v2.8.4
Python: 3.9.7 (tags/v3.9.7:1016ef3, Aug 30 2021, 20:19:38) [MSC v.1929 64 bit (AMD64)]
OS: Windows 10
  • Install method: pipx

Current Behavior / Repro Steps / Expected behavior

The list of formats outputted by jrnl is the same whether we're talking about importing or exporting. To see this in action, enter in a nonsensical import format like so:

>jrnl --import --format thiswillbreak
usage: jrnl [--debug] [--help] [--version] [--list] [--encrypt] [--decrypt] [--import] [-on DATE] [-today-in-history] [-month DATE] [-day DATE]
            [-year DATE] [-from DATE] [-to DATE] [-contains TEXT] [-and] [-starred] [-n [NUMBER]] [-not [TAG]] [--edit] [--delete]
            [--format TYPE] [--tags] [--short] [--config-override CONFIG_KV_PAIR CONFIG_KV_PAIR] [--config-file CONFIG_FILE_PATH]
            [...]
jrnl: error: argument --format: invalid choice: 'thiswillbreak' (choose from 'boxed', 'dates', 'fancy', 'json', 'markdown', 'md', 'pretty', 'sample', 'short', 'tags', 'text', 'txt', 'xml', 'yaml')

All those formats that jrnl offers are export formats and none are import formats (the only import format is jrnl), so that message shouldn't appear.

If you try any of those formats, jrnl crashes with a traceback like so:

>jrnl --import --format json
Traceback (most recent call last):
  File "C:\Users\micah\.pyenv\pyenv-win\versions\3.9.7\lib\runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\micah\.pyenv\pyenv-win\versions\3.9.7\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "C:\Users\micah\.local\bin\jrnl.exe\__main__.py", line 7, in <module>
  File "C:\Users\micah\.local\pipx\venvs\jrnl\lib\site-packages\jrnl\cli.py", line 34, in cli
    return run(args)
  File "C:\Users\micah\.local\pipx\venvs\jrnl\lib\site-packages\jrnl\jrnl.py", line 53, in run
    return args.postconfig_cmd(
  File "C:\Users\micah\.local\pipx\venvs\jrnl\lib\site-packages\jrnl\commands.py", line 57, in postconfig_import
    get_importer(format).import_(journal, args.filename)
AttributeError: 'NoneType' object has no attribute 'import_'

It should show a user-friendly error message instead, probably explaining that this is only an export format, not an import format.

Other Information

Found this out after spotting discussion #1476.

@micahellison micahellison added bug Something isn't working 🆕 New! ready for pr Okay to start work. Feel free to ask questions. and removed 🆕 New! labels May 23, 2022
@micahellison micahellison added this to the Backlog milestone May 28, 2022
pjz added a commit to pjz/jrnl that referenced this issue Aug 23, 2022
Instead of --import paying attention to --format, leave --format
just for the export-format and give --import itself an optional
arguent that is the import format.  This of course defaults to 'jrnl'.
This allows argparse to do the choice-checking.
@pjz pjz mentioned this issue Aug 23, 2022
4 tasks
@edoardob90 edoardob90 linked a pull request Nov 17, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready for pr Okay to start work. Feel free to ask questions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant