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

Partial refactor of cli.py (mainly help screen and arg parsing) #991

Merged
merged 17 commits into from Aug 1, 2020

Conversation

wren
Copy link
Member

@wren wren commented Jul 4, 2020

Fixes #344
Fixes #374
Fixes #520
Fixes #814
Fixes #866
Fixes #875

This is a partial refactor (another PR to come soon) cli.py, specifically focused on the
arg parsing code, and the run function. For any args that were changed, I also added
the old flag as a hidden option, so existing aliases and scripts will still work behind
the scenes.

Most of the changes are transparent to users, but will make life easier for
contributors.

Changes that affect users:

  • Fixed some bugs (see above)
  • Reorganized the help screen (might be too big now, so maybe it's the new man page?)
  • Changed the name of some flags (old flags still work)
  • Some flags operate slightly differently. Instead of taking an optional string
    immediately after the argument (e.g. --encrypt filename.txt), they now take that
    optional string from an option (e.g. --encrypt --file filename.txt). Please note
    that these additional flags are completely optional, and not giving these options has
    the same default behavior as before. The affected flags are:
    • --import (--import TYPE is now --import --format TYPE)
    • --encrypt (--encrypt FILENAME is now --encrypt --file FILENAME)
    • --decrypt (--decrypt FILENAME is now --decrypt --file FILENAME)

Changes that don't affect users, but do affect contributors:

  • Standalone commands (like --list and --encrypt) are now housed in an argparse
    option, and are executed at the correct time. This makes it easier to add more
    commands later (commands.py), and reduces the amount of overhead tracking all the
    various command line options that a user can specify. You can now add a standalone
    command by simply writing a function (commands.py), and registering the function
    in the arg parser (arg_parser.py). That's it. Then jrnl --newcommand will work as
    expected.
  • The multiple modes of jrnl are disappearing, and we'll be left only with searching,
    and writing. This PR starts that work (although it's not complete). After searching,
    you have the option to do various things with the search results (e.g. put them into
    editor, delete them, display them in alternate format, etc). That's already how jrnl
    works for the users, but the code will now better follow that flow.
  • In this PR, more functions have gone into util.py (I know, I'm sorry). The intent is
    not for them to stay there (we already have an issue to clean that file up Pull functionality out of util.py #737). The
    functions here will be split into smaller files when that issue is dealt with.
  • More tests! Mainly starting to flesh out the pytest suite, but also adding some more
    behave tests.
  • Our behave now supports simulating piping data into jrnl. This was added for testing
    importing, but can be used elsewhere as needed.

Checklist

  • I have read the contributing doc.
  • I have included a link to the relevant issue number.
  • I have tested this code locally.
  • I have checked to ensure there aren't other open pull requests
    for the same issue.
  • I have written new tests for these changes, as needed.
  • All tests pass.

@wren wren changed the title Start refactor of cli and arg parsing Partial refactor of cli.py (mainly help screen and arg parsing) Jul 4, 2020
wren added 15 commits August 1, 2020 15:04
Fixes jrnl-org#520 -and parameter seems to only work for the default journal
Also, cleaned up the way the arg parser handles standalone commands.
Rather than checking individually for each command, you can now register
the command in the proper place, and it will be run with all known
arguments (and cofig if available).
It's now part of args, and it's parsed into a function after the config
is available
If a date was given with an entry, and the star was also was added, the
star wouldn't be recognized if it was at the start of the title.

Example that didn't work, but now works with this fix:
  jrnl "saturday: *Title words."

This is to be consistent in starring functionality with and without a
date in the entry.
If a date was given with an entry, and the star was also was added, the
star wouldn't be recognized if it was at the start of the title.

Example that didn't work, but now works with this fix:
  jrnl "saturday: *Title words."

This is to be consistent in starring functionality with and without a
date in the entry.
Copy link
Member

@micahellison micahellison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great tests, nice refactoring, and it's a delight to see these argument bugs squashed. This will be nice to build off of. Thanks for your work on this.

@wren wren merged commit 3242993 into jrnl-org:develop Aug 1, 2020
@wren wren deleted the arg-parsing-redo-866 branch August 1, 2020 22:54
@wren wren added the bug Something isn't working label Oct 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants