diff --git a/command.py b/command.py index 8c5e2461..f7d20a22 100644 --- a/command.py +++ b/command.py @@ -98,6 +98,16 @@ class Command(object): self.OptionParser.print_usage() sys.exit(1) + def ValidateOptions(self, opt, args): + """Validate the user options & arguments before executing. + + This is meant to help break the code up into logical steps. Some tips: + * Use self.OptionParser.error to display CLI related errors. + * Adjust opt member defaults as makes sense. + * Adjust the args list, but do so inplace so the caller sees updates. + * Try to avoid updating self state. Leave that to Execute. + """ + def Execute(self, opt, args): """Perform the action, after option parsing is complete. """ diff --git a/main.py b/main.py index 889fc216..2ab79b57 100755 --- a/main.py +++ b/main.py @@ -197,6 +197,7 @@ class _Repo(object): cmd_event = cmd.event_log.Add(name, event_log.TASK_COMMAND, start) cmd.event_log.SetParent(cmd_event) try: + cmd.ValidateOptions(copts, cargs) result = cmd.Execute(copts, cargs) except (DownloadError, ManifestInvalidRevisionError, NoManifestException) as e: diff --git a/subcmds/abandon.py b/subcmds/abandon.py index 319262bd..cd1d0c40 100644 --- a/subcmds/abandon.py +++ b/subcmds/abandon.py @@ -37,19 +37,19 @@ It is equivalent to "git branch -D ". dest='all', action='store_true', help='delete all branches in all projects') - def Execute(self, opt, args): + def ValidateOptions(self, opt, args): if not opt.all and not args: self.Usage() if not opt.all: nb = args[0] if not git.check_ref_format('heads/%s' % nb): - print("error: '%s' is not a valid name" % nb, file=sys.stderr) - sys.exit(1) + self.OptionParser.error("'%s' is not a valid branch name" % nb) else: - args.insert(0,None) - nb = "'All local branches'" + args.insert(0, "'All local branches'") + def Execute(self, opt, args): + nb = args[0] err = defaultdict(list) success = defaultdict(list) all_projects = self.GetProjects(args[1:]) diff --git a/subcmds/checkout.py b/subcmds/checkout.py index 51ac4833..c8a09a8e 100644 --- a/subcmds/checkout.py +++ b/subcmds/checkout.py @@ -34,10 +34,11 @@ The command is equivalent to: repo forall [...] -c git checkout """ - def Execute(self, opt, args): + def ValidateOptions(self, opt, args): if not args: self.Usage() + def Execute(self, opt, args): nb = args[0] err = [] success = [] diff --git a/subcmds/cherry_pick.py b/subcmds/cherry_pick.py index 43215f91..a541a040 100644 --- a/subcmds/cherry_pick.py +++ b/subcmds/cherry_pick.py @@ -37,10 +37,11 @@ change id will be added. def _Options(self, p): pass - def Execute(self, opt, args): + def ValidateOptions(self, opt, args): if len(args) != 1: self.Usage() + def Execute(self, opt, args): reference = args[0] p = GitCommand(None, diff --git a/subcmds/diffmanifests.py b/subcmds/diffmanifests.py index cae776a7..b999699e 100644 --- a/subcmds/diffmanifests.py +++ b/subcmds/diffmanifests.py @@ -176,10 +176,11 @@ synced and their revisions won't be found. self.printText(log) self.out.nl() - def Execute(self, opt, args): + def ValidateOptions(self, opt, args): if not args or len(args) > 2: - self.Usage() + self.OptionParser.error('missing manifests to diff') + def Execute(self, opt, args): self.out = _Coloring(self.manifest.globalConfig) self.printText = self.out.nofmt_printer('text') if opt.color: diff --git a/subcmds/forall.py b/subcmds/forall.py index 3e6007ba..0be8d3bc 100644 --- a/subcmds/forall.py +++ b/subcmds/forall.py @@ -177,10 +177,11 @@ without iterating through the remaining projects. 'worktree': project.worktree, } - def Execute(self, opt, args): + def ValidateOptions(self, opt, args): if not opt.command: self.Usage() + def Execute(self, opt, args): cmd = [opt.command[0]] shell = True diff --git a/subcmds/init.py b/subcmds/init.py index eaa6da50..32663a04 100644 --- a/subcmds/init.py +++ b/subcmds/init.py @@ -436,18 +436,17 @@ to update the working directory files. print(' rm -r %s/.repo' % self.manifest.topdir) print('and try again.') - def Execute(self, opt, args): - git_require(MIN_GIT_VERSION, fail=True) - + def ValidateOptions(self, opt, args): if opt.reference: opt.reference = os.path.expanduser(opt.reference) # Check this here, else manifest will be tagged "not new" and init won't be # possible anymore without removing the .repo/manifests directory. if opt.archive and opt.mirror: - print('fatal: --mirror and --archive cannot be used together.', - file=sys.stderr) - sys.exit(1) + self.OptionParser.error('--mirror and --archive cannot be used together.') + + def Execute(self, opt, args): + git_require(MIN_GIT_VERSION, fail=True) self._SyncManifest(opt) self._LinkManifest(opt.manifest_name) diff --git a/subcmds/list.py b/subcmds/list.py index 961b1956..00172f0e 100644 --- a/subcmds/list.py +++ b/subcmds/list.py @@ -49,6 +49,10 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'. dest='path_only', action='store_true', help="Display only the path of the repository") + def ValidateOptions(self, opt, args): + if opt.fullpath and opt.name_only: + self.OptionParser.error('cannot combine -f and -n') + def Execute(self, opt, args): """List all projects and the associated directories. @@ -60,11 +64,6 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'. opt: The options. args: Positional args. Can be a list of projects to list, or empty. """ - - if opt.fullpath and opt.name_only: - print('error: cannot combine -f and -n', file=sys.stderr) - sys.exit(1) - if not opt.regex: projects = self.GetProjects(args, groups=opt.groups) else: diff --git a/subcmds/manifest.py b/subcmds/manifest.py index 07fa323a..768f072e 100644 --- a/subcmds/manifest.py +++ b/subcmds/manifest.py @@ -73,14 +73,9 @@ in a Git repository for use during future 'repo init' invocations. if opt.output_file != '-': print('Saved manifest to %s' % opt.output_file, file=sys.stderr) - def Execute(self, opt, args): + def ValidateOptions(self, opt, args): if args: self.Usage() - if opt.output_file is not None: - self._Output(opt) - return - - print('error: no operation to perform', file=sys.stderr) - print('error: see repo help manifest', file=sys.stderr) - sys.exit(1) + def Execute(self, opt, args): + self._Output(opt) diff --git a/subcmds/start.py b/subcmds/start.py index 0c60d78c..5d4c9c01 100644 --- a/subcmds/start.py +++ b/subcmds/start.py @@ -41,15 +41,16 @@ revision specified in the manifest. dest='all', action='store_true', help='begin branch in all projects') - def Execute(self, opt, args): + def ValidateOptions(self, opt, args): if not args: self.Usage() nb = args[0] if not git.check_ref_format('heads/%s' % nb): - print("error: '%s' is not a valid name" % nb, file=sys.stderr) - sys.exit(1) + self.OptionParser.error("'%s' is not a valid name" % nb) + def Execute(self, opt, args): + nb = args[0] err = [] projects = [] if not opt.all: diff --git a/subcmds/sync.py b/subcmds/sync.py index 3eab2fcf..5655a1e6 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -738,6 +738,24 @@ later is required to fix a server side protocol bug. fd.close() return 0 + def ValidateOptions(self, opt, args): + if opt.force_broken: + print('warning: -f/--force-broken is now the default behavior, and the ' + 'options are deprecated', file=sys.stderr) + if opt.network_only and opt.detach_head: + self.OptionParser.error('cannot combine -n and -d') + if opt.network_only and opt.local_only: + self.OptionParser.error('cannot combine -n and -l') + if opt.manifest_name and opt.smart_sync: + self.OptionParser.error('cannot combine -m and -s') + if opt.manifest_name and opt.smart_tag: + self.OptionParser.error('cannot combine -m and -t') + if opt.manifest_server_username or opt.manifest_server_password: + if not (opt.smart_sync or opt.smart_tag): + self.OptionParser.error('-u and -p may only be combined with -s or -t') + if None in [opt.manifest_server_username, opt.manifest_server_password]: + self.OptionParser.error('both -u and -p must be given') + def Execute(self, opt, args): if opt.jobs: self.jobs = opt.jobs @@ -745,30 +763,6 @@ later is required to fix a server side protocol bug. soft_limit, _ = _rlimit_nofile() self.jobs = min(self.jobs, (soft_limit - 5) // 3) - if opt.force_broken: - print('warning: -f/--force-broken is now the default behavior, and the ' - 'options are deprecated', file=sys.stderr) - if opt.network_only and opt.detach_head: - print('error: cannot combine -n and -d', file=sys.stderr) - sys.exit(1) - if opt.network_only and opt.local_only: - print('error: cannot combine -n and -l', file=sys.stderr) - sys.exit(1) - if opt.manifest_name and opt.smart_sync: - print('error: cannot combine -m and -s', file=sys.stderr) - sys.exit(1) - if opt.manifest_name and opt.smart_tag: - print('error: cannot combine -m and -t', file=sys.stderr) - sys.exit(1) - if opt.manifest_server_username or opt.manifest_server_password: - if not (opt.smart_sync or opt.smart_tag): - print('error: -u and -p may only be combined with -s or -t', - file=sys.stderr) - sys.exit(1) - if None in [opt.manifest_server_username, opt.manifest_server_password]: - print('error: both -u and -p must be given', file=sys.stderr) - sys.exit(1) - if opt.manifest_name: self.manifest.Override(opt.manifest_name)