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鈥檒l 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

techknowlogick commented Feb 10, 2020

馃憢 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
鈥tion and bash complete
@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 robjloranger Feb 22, 2020
@thebaer thebaer added this to the 0.12 milestone Mar 2, 2020
Copy link
Member

thebaer left a comment

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

techknowlogick commented Mar 2, 2020

@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 approved these changes Mar 2, 2020
@thebaer thebaer merged commit c71d020 into writeas:develop Mar 2, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@techknowlogick techknowlogick deleted the techknowlogick:update-cli branch Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can鈥檛 perform that action at this time.