-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Implement our fancy new --help output #3883
Conversation
- Currently exceptions are often caught and burried in log files, even if this flag is provided!
* Parallalelise nosetests from tox * Parallelise even more things, break even more things * Now unbreak all the tests that aren't ready for ||ism * Try to pass tests! - Remove non-working hack in reporter_test - also be selective about ||ism in the cover environment * Try again * certbot-apache tests also work, given enough time * Nginx may need more time in Travis's cloud * Unbreak reporter_test under ||ism * More timeout * Working again? * This goes way faster * Another big win * Split a couple more large test suites * A last improvement * More ||ism! * ||ise lint too * Allow nosetests to figure out how many cores to use * simplify merge * Mark the new CLI tests as ||izable * Simplify reporter_test changes * Rationalise ||ism flags * Re-up coverage * Clean up reporter tests * Stop modifying testdata during tests * remove unused os
* Begin making "certbot certificates" future safe * Handle the case where a renewal conf file has no "server" entry
Also, update nginx not installed CLI hint
- subcommands now get their own usage headings if they want them - added "certbot -h commands"
This includes #3877 only because I was using it while writing this branch, but it's going to be hard to rebase out due to subsequent merges... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Niiiice! Were we going to add a manage
help group, or did we decide not to, or is that coming in a future PR?
--authenticator standalone --installer apache | ||
manage certificates: | ||
certificates Display information about certs you have from Certbot | ||
revoke Revoke a certificate (supply --cert-path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
"short": "Revoke a certificate specified with --cert-path", | ||
"opts": "Options for revocation of certs" | ||
}), | ||
("revoke", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"rename" not "revoke"
for verb, props in VERB_HELP: | ||
padding = (longest - len(verb)) + 4 | ||
doc = props.get("short", "") | ||
text += verb + " " * padding + doc + "\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For extra fancy: text += '{0:<{length}} {1}\n'.format(verb, doc, length=longest)
# both for "cerbot -h SUBCOMMAND" and "certbot -h all" | ||
# usage: an optional string that overrides the header of "certbot -h SUBCOMMAND" | ||
VERB_HELP = [ | ||
("run (default)", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we alphabetize these, now that they're available on more of a reference page than a "most important first" help page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
}), | ||
("update_symlinks", { | ||
"short": "Recreate symlinks in your /live/ directory", | ||
"opts": ("Recreates cert and key symlinks in {0}, if you changed them by hand, " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: these commas do not belong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at a grammatical ruleset about commas I believe the first one is advisable under rule 4A; the second is permitted under rule 3A though perhaps you could argue that the two clauses are sufficiently short that it could be omitted :)
|
I thought we were just going to have |
You're totally right... |
Also, implement a general mechanism for documenting subcommands within topics
Ok I added a
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One typo that needs to be fixed, one opinion that doesn't. Otherwise looks great, I'll let you merge!
@@ -788,15 +795,15 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis | |||
"multiple -d flags or enter a comma separated list of domains " | |||
"as a parameter.") | |||
helpful.add( | |||
[None, "run", "certonly"], | |||
[None, "run", "certonly", "manage"], | |||
"--cert-name", dest="certname", | |||
metavar="CERTNAME", default=None, | |||
help="Certificate name to apply. Only one certificate name can be used " | |||
"per Certbot run. To see certificate names, run 'certbot certificates'." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a space after the period
@@ -1051,9 +1058,7 @@ def _paths_parser(helpful): | |||
verb = helpful.help_arg | |||
|
|||
cph = "Path to where cert is saved (with auth --csr), installed from or revoked." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that a lack of oxford comma I see? Probably because other parts are hogging all the commas.
The top level help is now:
There is also a
cerbot -h commands
, which produces:It's also possible to customise the help more for any given subcommand; I've done that for
run
,certonly
, andrenew
. For instance,certbot -h renew
now produces: