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

Refactor CLI #261

Merged
merged 4 commits into from Mar 2, 2020
Merged

Refactor CLI #261

merged 4 commits into from Mar 2, 2020

Conversation

techknowlogick
Copy link
Contributor

👋 Hey, I know this is kinda a big PR and I'm thankful for you taking the time to even consider it. First, I want to say I understand that if this doesn't align with the current goal of the project that it may be closed, and that's totally ok with me.

Currently the CLI uses flags for both commands and options, this refactor switches it out to a format that allows for bashcompletion built in, and can auto-generate manpages.

For legacy sake, previous style of working with CLI still exists.

If the previous style of using only flags for everything is preferable, then this PR can be changed to still use urfave/cli, but still be flag only and would still provide the benefits of the refactored code style.

Please let me know if you have any questions.

Example help output:

NAME:
   WriteFreely - A beautifully pared-down blogging platform

USAGE:
   writefreely [global options] command [command options] [arguments...]

VERSION:
   WriteFreely 0.11.2

COMMANDS:
   user        user management tools
   db          db management tools
   config      config management tools
   keys        key management tools
   serve, web  Run web application
   help, h     Shows a list of commands or help for one command

GLOBAL OPTIONS:
   -c FILE        Load configuration from FILE (default: "config.ini")
   --debug        Enables debug logging (default: false)
   --help, -h     show help (default: false)
   --version, -v  print the version (default: false)

  • I have signed the CLA

@thebaer
Copy link
Member

thebaer commented Feb 15, 2020

Thanks for working on this! It'll still be a while before I can fully test / review, but wanted to mention that I am definitely interested in merging this.

@thebaer thebaer requested a review from a user February 22, 2020 15:31
@thebaer thebaer added this to the 0.12 milestone Mar 2, 2020
Copy link
Member

@thebaer thebaer left a comment

Choose a reason for hiding this comment

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

Thanks again for making these changes! Looking forward to getting this merged.

Overall everything seems great, functionally -- and I appreciate the effort to keep this backward-compatible! I mostly propose some naming changes, and noticed some examples that need updating. Otherwise, might need to run go fmt on some files. With those changes, this should be good to go!

cmd/writefreely/user.go Outdated Show resolved Hide resolved
cmd/writefreely/config.go Outdated Show resolved Hide resolved
cmd/writefreely/config.go Outdated Show resolved Hide resolved
cmd/writefreely/config.go Outdated Show resolved Hide resolved
cmd/writefreely/user.go Outdated Show resolved Hide resolved
cmd/writefreely/main.go Show resolved Hide resolved
@techknowlogick
Copy link
Contributor Author

@thebaer thanks for feedback. I've just pushed requested changes :)

@thebaer
Copy link
Member

thebaer commented Mar 2, 2020

Thanks for the quick fixes! Looks great 👍 merging now.

@thebaer thebaer merged commit c71d020 into writefreely:develop Mar 2, 2020
@techknowlogick techknowlogick deleted the update-cli branch March 2, 2020 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants