diff --git a/subcmds/diff.py b/subcmds/diff.py index 4966bb1a..b400ccfd 100644 --- a/subcmds/diff.py +++ b/subcmds/diff.py @@ -33,7 +33,7 @@ to the Unix 'patch' command. def _Options(self, p): p.add_option('-u', '--absolute', dest='absolute', action='store_true', - help='Paths are relative to the repository root') + help='paths are relative to the repository root') def _ExecuteOne(self, absolute, project): """Obtains the diff for a specific project. diff --git a/subcmds/diffmanifests.py b/subcmds/diffmanifests.py index 392e5972..6f23b345 100644 --- a/subcmds/diffmanifests.py +++ b/subcmds/diffmanifests.py @@ -68,10 +68,10 @@ synced and their revisions won't be found. def _Options(self, p): p.add_option('--raw', dest='raw', action='store_true', - help='Display raw diff.') + help='display raw diff') p.add_option('--no-color', dest='color', action='store_false', default=True, - help='does not display the diff in color.') + help='does not display the diff in color') p.add_option('--pretty-format', dest='pretty_format', action='store', metavar='', diff --git a/subcmds/forall.py b/subcmds/forall.py index 4a631fb7..0cf3b6a6 100644 --- a/subcmds/forall.py +++ b/subcmds/forall.py @@ -131,30 +131,30 @@ without iterating through the remaining projects. def _Options(self, p): p.add_option('-r', '--regex', dest='regex', action='store_true', - help="Execute the command only on projects matching regex or wildcard expression") + help='execute the command only on projects matching regex or wildcard expression') p.add_option('-i', '--inverse-regex', dest='inverse_regex', action='store_true', - help="Execute the command only on projects not matching regex or " - "wildcard expression") + help='execute the command only on projects not matching regex or ' + 'wildcard expression') p.add_option('-g', '--groups', dest='groups', - help="Execute the command only on projects matching the specified groups") + help='execute the command only on projects matching the specified groups') p.add_option('-c', '--command', - help='Command (and arguments) to execute', + help='command (and arguments) to execute', dest='command', action='callback', callback=self._cmd_option) p.add_option('-e', '--abort-on-errors', dest='abort_on_errors', action='store_true', - help='Abort if a command exits unsuccessfully') + help='abort if a command exits unsuccessfully') p.add_option('--ignore-missing', action='store_true', - help='Silently skip & do not exit non-zero due missing ' + help='silently skip & do not exit non-zero due missing ' 'checkouts') g = p.get_option_group('--quiet') g.add_option('-p', dest='project_header', action='store_true', - help='Show project headers before output') + help='show project headers before output') p.add_option('--interactive', action='store_true', help='force interactive usage') diff --git a/subcmds/gitc_delete.py b/subcmds/gitc_delete.py index 56e0eaba..54b956f7 100644 --- a/subcmds/gitc_delete.py +++ b/subcmds/gitc_delete.py @@ -33,7 +33,7 @@ and all locally downloaded sources. def _Options(self, p): p.add_option('-f', '--force', dest='force', action='store_true', - help='Force the deletion (no prompt).') + help='force the deletion (no prompt)') def Execute(self, opt, args): if not opt.force: diff --git a/subcmds/info.py b/subcmds/info.py index 2be56109..f7cf60fc 100644 --- a/subcmds/info.py +++ b/subcmds/info.py @@ -48,7 +48,7 @@ class Info(PagedCommand): help=optparse.SUPPRESS_HELP) p.add_option('-l', '--local-only', dest="local", action="store_true", - help="Disable all remote operations") + help="disable all remote operations") def Execute(self, opt, args): self.out = _Coloring(self.client.globalConfig) diff --git a/subcmds/list.py b/subcmds/list.py index 5cbc0c22..68bcd5e0 100644 --- a/subcmds/list.py +++ b/subcmds/list.py @@ -36,22 +36,22 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'. def _Options(self, p): p.add_option('-r', '--regex', dest='regex', action='store_true', - help="Filter the project list based on regex or wildcard matching of strings") + help='filter the project list based on regex or wildcard matching of strings') p.add_option('-g', '--groups', dest='groups', - help="Filter the project list based on the groups the project is in") + help='filter the project list based on the groups the project is in') p.add_option('-a', '--all', action='store_true', - help='Show projects regardless of checkout state') + help='show projects regardless of checkout state') p.add_option('-f', '--fullpath', dest='fullpath', action='store_true', - help="Display the full work tree path instead of the relative path") + help='display the full work tree path instead of the relative path') p.add_option('-n', '--name-only', dest='name_only', action='store_true', - help="Display only the name of the repository") + help='display only the name of the repository') p.add_option('-p', '--path-only', dest='path_only', action='store_true', - help="Display only the path of the repository") + help='display only the path of the repository') def ValidateOptions(self, opt, args): if opt.fullpath and opt.name_only: diff --git a/subcmds/manifest.py b/subcmds/manifest.py index e33e683c..965c36e9 100644 --- a/subcmds/manifest.py +++ b/subcmds/manifest.py @@ -53,27 +53,27 @@ to indicate the remote ref to push changes to via 'repo upload'. def _Options(self, p): p.add_option('-r', '--revision-as-HEAD', dest='peg_rev', action='store_true', - help='Save revisions as current HEAD') + help='save revisions as current HEAD') p.add_option('-m', '--manifest-name', help='temporary manifest to use for this sync', metavar='NAME.xml') p.add_option('--suppress-upstream-revision', dest='peg_rev_upstream', default=True, action='store_false', - help='If in -r mode, do not write the upstream field. ' - 'Only of use if the branch names for a sha1 manifest are ' - 'sensitive.') + help='if in -r mode, do not write the upstream field ' + '(only of use if the branch names for a sha1 manifest are ' + 'sensitive)') p.add_option('--suppress-dest-branch', dest='peg_rev_dest_branch', default=True, action='store_false', - help='If in -r mode, do not write the dest-branch field. ' - 'Only of use if the branch names for a sha1 manifest are ' - 'sensitive.') + help='if in -r mode, do not write the dest-branch field ' + '(only of use if the branch names for a sha1 manifest are ' + 'sensitive)') p.add_option('--json', default=False, action='store_true', - help='Output manifest in JSON format (experimental).') + help='output manifest in JSON format (experimental)') p.add_option('--pretty', default=False, action='store_true', - help='Format output for humans to read.') + help='format output for humans to read') p.add_option('-o', '--output-file', dest='output_file', default='-', - help='File to save the manifest to', + help='file to save the manifest to', metavar='-|NAME.xml') def _Output(self, opt): diff --git a/subcmds/overview.py b/subcmds/overview.py index 202a5eba..b28367be 100644 --- a/subcmds/overview.py +++ b/subcmds/overview.py @@ -36,7 +36,7 @@ are displayed. def _Options(self, p): p.add_option('-c', '--current-branch', dest="current_branch", action="store_true", - help="Consider only checked out branches") + help="consider only checked out branches") p.add_option('--no-current-branch', dest='current_branch', action='store_false', help='consider all local branches') diff --git a/subcmds/rebase.py b/subcmds/rebase.py index e0186d4d..9ce4ecb8 100644 --- a/subcmds/rebase.py +++ b/subcmds/rebase.py @@ -46,27 +46,27 @@ branch but need to incorporate new upstream changes "underneath" them. p.add_option('--fail-fast', dest='fail_fast', action='store_true', - help='Stop rebasing after first error is hit') + help='stop rebasing after first error is hit') p.add_option('-f', '--force-rebase', dest='force_rebase', action='store_true', - help='Pass --force-rebase to git rebase') + help='pass --force-rebase to git rebase') p.add_option('--no-ff', dest='ff', default=True, action='store_false', - help='Pass --no-ff to git rebase') + help='pass --no-ff to git rebase') p.add_option('--autosquash', dest='autosquash', action='store_true', - help='Pass --autosquash to git rebase') + help='pass --autosquash to git rebase') p.add_option('--whitespace', dest='whitespace', action='store', metavar='WS', - help='Pass --whitespace to git rebase') + help='pass --whitespace to git rebase') p.add_option('--auto-stash', dest='auto_stash', action='store_true', - help='Stash local modifications before starting') + help='stash local modifications before starting') p.add_option('-m', '--onto-manifest', dest='onto_manifest', action='store_true', - help='Rebase onto the manifest version instead of upstream ' - 'HEAD. This helps to make sure the local tree stays ' - 'consistent if you previously synced to a manifest.') + help='rebase onto the manifest version instead of upstream ' + 'HEAD (this helps to make sure the local tree stays ' + 'consistent if you previously synced to a manifest)') def Execute(self, opt, args): all_projects = self.GetProjects(args) diff --git a/subcmds/sync.py b/subcmds/sync.py index e5280865..31760e5c 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -169,10 +169,11 @@ later is required to fix a server side protocol bug. PARALLEL_JOBS = 1 def _CommonOptions(self, p): - try: - self.PARALLEL_JOBS = self.manifest.default.sync_j - except ManifestParseError: - pass + if self.manifest: + try: + self.PARALLEL_JOBS = self.manifest.default.sync_j + except ManifestParseError: + pass super()._CommonOptions(p) def _Options(self, p, show_smart=True): diff --git a/subcmds/upload.py b/subcmds/upload.py index c497d877..fc389e42 100644 --- a/subcmds/upload.py +++ b/subcmds/upload.py @@ -152,61 +152,61 @@ Gerrit Code Review: https://www.gerritcodereview.com/ def _Options(self, p): p.add_option('-t', dest='auto_topic', action='store_true', - help='Send local branch name to Gerrit Code Review') + help='send local branch name to Gerrit Code Review') p.add_option('--hashtag', '--ht', dest='hashtags', action='append', default=[], - help='Add hashtags (comma delimited) to the review.') + help='add hashtags (comma delimited) to the review') p.add_option('--hashtag-branch', '--htb', action='store_true', - help='Add local branch name as a hashtag.') + help='add local branch name as a hashtag') p.add_option('-l', '--label', dest='labels', action='append', default=[], - help='Add a label when uploading.') + help='add a label when uploading') p.add_option('--re', '--reviewers', type='string', action='append', dest='reviewers', - help='Request reviews from these people.') + help='request reviews from these people') p.add_option('--cc', type='string', action='append', dest='cc', - help='Also send email to these email addresses.') + help='also send email to these email addresses') p.add_option('--br', '--branch', type='string', action='store', dest='branch', - help='(Local) branch to upload.') + help='(local) branch to upload') p.add_option('-c', '--current-branch', dest='current_branch', action='store_true', - help='Upload current git branch.') + help='upload current git branch') p.add_option('--no-current-branch', dest='current_branch', action='store_false', - help='Upload all git branches.') + help='upload all git branches') # Turn this into a warning & remove this someday. p.add_option('--cbr', dest='current_branch', action='store_true', help=optparse.SUPPRESS_HELP) p.add_option('--ne', '--no-emails', action='store_false', dest='notify', default=True, - help='If specified, do not send emails on upload.') + help='do not send e-mails on upload') p.add_option('-p', '--private', action='store_true', dest='private', default=False, - help='If specified, upload as a private change.') + help='upload as a private change (deprecated; use --wip)') p.add_option('-w', '--wip', action='store_true', dest='wip', default=False, - help='If specified, upload as a work-in-progress change.') + help='upload as a work-in-progress change') p.add_option('-o', '--push-option', type='string', action='append', dest='push_options', default=[], - help='Additional push options to transmit') + help='additional push options to transmit') p.add_option('-D', '--destination', '--dest', type='string', action='store', dest='dest_branch', metavar='BRANCH', - help='Submit for review on this target branch.') + help='submit for review on this target branch') p.add_option('-n', '--dry-run', dest='dryrun', default=False, action='store_true', - help='Do everything except actually upload the CL.') + help='do everything except actually upload the CL') p.add_option('-y', '--yes', default=False, action='store_true', - help='Answer yes to all safe prompts.') + help='answer yes to all safe prompts') p.add_option('--no-cert-checks', dest='validate_certs', action='store_false', default=True, - help='Disable verifying ssl certs (unsafe).') + help='disable verifying ssl certs (unsafe)') RepoHook.AddOptionGroup(p, 'pre-upload') def _SingleBranch(self, opt, branch, people): diff --git a/tests/test_subcmds.py b/tests/test_subcmds.py index 2234e646..bc53051a 100644 --- a/tests/test_subcmds.py +++ b/tests/test_subcmds.py @@ -14,6 +14,7 @@ """Unittests for the subcmds module (mostly __init__.py than subcommands).""" +import optparse import unittest import subcmds @@ -41,3 +42,32 @@ class AllCommands(unittest.TestCase): # Reject internal python paths like "__init__". self.assertFalse(cmd.startswith('__')) + + def test_help_desc_style(self): + """Force some consistency in option descriptions. + + Python's optparse & argparse has a few default options like --help. Their + option description text uses lowercase sentence fragments, so enforce our + options follow the same style so UI is consistent. + + We enforce: + * Text starts with lowercase. + * Text doesn't end with period. + """ + for name, cls in subcmds.all_commands.items(): + cmd = cls() + parser = cmd.OptionParser + for option in parser.option_list: + if option.help == optparse.SUPPRESS_HELP: + continue + + c = option.help[0] + self.assertEqual( + c.lower(), c, + msg=f'subcmds/{name}.py: {option.get_opt_string()}: help text ' + f'should start with lowercase: "{option.help}"') + + self.assertNotEqual( + option.help[-1], '.', + msg=f'subcmds/{name}.py: {option.get_opt_string()}: help text ' + f'should not end in a period: "{option.help}"')