Argparse "Overlay" Module #12873
Argparse "Overlay" Module #12873
Conversation
Rostepher
force-pushed
the
argparse-module
branch
from
49db8c3 to
27c8b9f
Compare
There was a problem hiding this comment.
Other than adding docstrings explaining how to use this new argument parser shim and what is happening in this module, everything here looks like a solid addition to our argument parsing logic. As long as the changes going forward don't break backwards compatibility with our build presets, then this looks good to me.
| # See https://swift.org/LICENSE.txt for license information | ||
| # See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
|
|
||
|
|
There was a problem hiding this comment.
Please add a docstring explaining what this module is intended to do so it is used appropriately in the future :)
There was a problem hiding this comment.
No problem, I'll add those in a bit.
| # See https://swift.org/LICENSE.txt for license information | ||
| # See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
|
|
||
|
|
|
@rintaro If you have the time would you mind taking a look at this PR? |
|
In case anyone is wondering the unit-tests can be run via the command |
Rostepher
force-pushed
the
argparse-module
branch
from
77a525e to
fe9ecb3
Compare
|
@swift-ci please test |
|
@swift-ci please smoke test |
|
@swift-ci please Python lint |
|
@swift-ci please test OSX |
|
Build failed |
Sorry if I'm missing something -- have you thought about running these tests automatically, by adding a file like this one? |
|
I'm hesitant to add these unit-tests to the main suite because they take ~2 minutes to run thanks to To elaborate, the argument parser tests attempt to verify every single preset is valid by running the arguments not only through the python parser, but also through |
…ean-like values and replaces the hard-coded boolean values in the _ToggleAction class.
…imports, so now the unit-tests will actually run as expected. The README has also been updated with a better command for executing the unit-test suite.
… to only take a single action or default to the store action.
…or generating argument parser tests.
Rostepher
force-pushed
the
argparse-module
branch
from
80a173b to
3b0dd5d
Compare
|
@swift-ci please test |
|
@swift-ci please smoke test |
|
Build failed |
|
Build failed |
|
@swift-ci please test |
|
@swift-ci please smoke test |
|
Build failed |
|
Build failed |
Rostepher
force-pushed
the
argparse-module
branch
from
16614b4 to
9926a62
Compare
|
@swift-ci please test |
|
@swift-ci please smoke test |
|
Build failed |
|
Build failed |
|
Once @zisko's changes are added, LGTM. |
|
LGTM |
Purpose
This PR is meant to supersede #11827. Rather than one large PR that introduces an entire DSL and refactors the very large argument parsing logic all at once, it seemed better to introduce the argument parsing DSL separately and then in follow-up PRs complete the refactoring. I've used @rintaro's work from #2190 to construct a wrapper module, or overlay if you will, around the standard python
argparsewhich exposes the exact same API with some extras on top. Specifically there's now a bunch of new action classes which handle multiple destinations, a new builder class on theArgumentParserand lastly a handful of new*Typeclasses.The entire module is thoroughly tested and I'll be following this up with subsequent PRs to convert the argument parser piece-wise.
rdar://34336890