CPLAT-16476 Allow args after separator to TestTool #364
Conversation
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
|
|
||
| final args = argResults.arguments; | ||
| final rest = argResults.rest; | ||
| var rCursor = 0; |
There was a problem hiding this comment.
Maybe a clearer name? e.g. argSeparatorPosition
There was a problem hiding this comment.
I'm a little torn on this. I originally had something like what you suggested, but then usages like this feel a bit more confusing:
if (args[aCursor] == rest[separatorArgPosition]) {
There was a problem hiding this comment.
Yeah, I see that. What about not abbreviating. argsCursor and restCursor. I see aCursor at a glance and read it as "an instance of Cursor". Or argsIndex and restIndex?
| final rest = argResults.rest; | ||
| var rCursor = 0; | ||
| for (var aCursor = 0; aCursor < args.length; aCursor++) { | ||
| // Iterate through the original args until we hit the first separator. |
There was a problem hiding this comment.
I'm having trouble following this. It seems like the only result of this is setting rCursor. But isn't rCursor just args.indexOf('--')?
There was a problem hiding this comment.
That would be the position of the separator within the original arguments list, which isn't necessarily the same as the position at which the separator should be restored in the rest arguments list, since the latter excludes flags and options that were parsed by the ArgParser. So for example:
final argParser = ArgParser()
..addOption('output')
..addFlag('verbose');
final argResults = argParser.parse(['foo', '--output', 'out', '--verbose', '--', 'bar', '--baz']);
print(argResults.arguments);
// ['foo', '--output', 'out', '--verbose', '--', 'bar', '--baz']
// In this list, separator index is 4
print(argResults.rest);
// ['foo', 'bar', '--baz']
// In this list, we want to restore the separator at index 1There was a problem hiding this comment.
OK, I get it. Ugh. It'd be better if ArgParser just didn't think it knew about '--' and included it.
evanweible-wf
dismissed stale reviews from alanknight-wk and todbachman-wf
via
183b8b4
| final rest = argResults.rest; | ||
| var rCursor = 0; | ||
| for (var aCursor = 0; aCursor < args.length; aCursor++) { | ||
| // Iterate through the original args until we hit the first separator. |
There was a problem hiding this comment.
OK, I get it. Ugh. It'd be better if ArgParser just didn't think it knew about '--' and included it.
|
|
||
| final args = argResults.arguments; | ||
| final rest = argResults.rest; | ||
| var rCursor = 0; |
There was a problem hiding this comment.
Yeah, I see that. What about not abbreviating. argsCursor and restCursor. I see aCursor at a glance and read it as "an instance of Cursor". Or argsIndex and restIndex?
|
QA +1
|
rmconsole6-wk
changed the title
|
@Workiva/release-management-p |
TestToolto allow arguments after a separator (--). These arguments will always be passed to thedart testprocess. The main use case for this is integration with IDE plugins that enable running tests directly from the IDE.FunctionToolto allow arguments after a separator (--). There isn't a strong reason to disallow this since the function tool could do anything it wants with those args (and now we have a concrete use case for just that).takeAllArgs(the arg mapper util used withCompoundTool) so that it now properly restores the first separator (--) if present in the original arguments list.