clayborg accepted this revision.
clayborg added inline comments.
================
Comment at: utils/clangdiag.py:117
+ disable(exe_ctx)
+ else:
+ getDiagtool(exe_ctx.GetTarget(), args.path[0])
----------------
hintonda wrote:
> clayborg wrote:
> > should probably verify that this 'diagtool' with:
> >
> > ```
> > elif args.subcommands == 'diagtool':
> > ```
> > and add an else that creates an error:
> > ```
> > else:
> > result.SetError("invalid subcommand '%s'" % (args.subcommands))
> > ```
> This is already handled by `argparser`, which will raise an exception if the
> first parameter isn't one of (enable, disable, daigtool). That's the benefit
> of using `subparsers`. So, by the time you get to this point, it must be
> "diagtool".
>
> However, I think I should probably verify the path past actually exists.
> Plus, I think I could add a better exception to alert the user how to fix the
> problem if diagtool couldn't be found in the callback. Suggestions welcome.
nice, I didn't realized that argparser would protect against that. Checking the
file exists would be a good idea.
https://reviews.llvm.org/D36347
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
