refactor kpackagetool away from stringy options
ClosedPublic

Authored by sitter on Aug 9 2017, 1:24 PM.

Details

Summary

A notable advantage of qcommandlineoption is that using the objects the
compiler makes sure that everyone is talking about the same option.
Previously kpackagetool would create the option objects in main but then
not use them to grab values out of the commandlineparser, instead relying
on the stringy representation of the option. This is a tad harder to
read but more importantly, it bypasses the compiler opening the code up for
typos. Namely one could do d->parser->value("hahs") while doing
d->parser->value(Options::hahs) will result in a build failure.

To preven this, refactor the entire option handling to use static option
instances from a new Options namespace as to let the compiler help us not
write typos.

In the future option handling could be additionally changed to parse all
options at once early on and construct an option struct or something to
reduce code clutter from calling isSet a million times.

Test Plan
  • all existing tests pass
  • --help still looks correct

Diff Detail

Repository
R290 KPackage
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter created this revision.Aug 9 2017, 1:24 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptAug 9 2017, 1:24 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
apol added a comment.Aug 9 2017, 3:39 PM

if that makes you happy, it makes me happy.
+1

apol accepted this revision.Fri, Sep 1, 12:02 PM

Won't hurt, benefits from some compiler checking so it's better.

This revision is now accepted and ready to land.Fri, Sep 1, 12:02 PM
This revision was automatically updated to reflect the committed changes.