Reimplement the ArgSplitter in Rust. #21814
Conversation
|
Note that the future PR to wire this up already works in a branch, but I wanted to break things up for easier reviewing. |
| @@ -662,9 +662,17 @@ class PyOptionId: | |||
| self, *components: str, scope: str | None = None, switch: str | None = None | |||
| ) -> None: ... | |||
|
|
|||
| class PySplitArgs: | |||
There was a problem hiding this comment.
It's possible that we won't need to access this in Python in the future, but for now I've implemented this, to make the port less invasive, and also so that I can run the existing Python tests against this new implementation.
| @@ -109,7 +109,7 @@ def _get_known_goal_scopes( | |||
| yield alias, si | |||
|
|
|||
| def split_args(self, args: Sequence[str]) -> SplitArgs: | |||
| """Split the specified arg list (or sys.argv if unspecified). | |||
| """Split the specified arg list. | |||
There was a problem hiding this comment.
The previous statement was false...
| // We disambiguate thus: If it is a known goal name, assume the user intended a goal. | ||
| // Otherwise, if it looks like a path or target, or exists on the filesystem, assume | ||
| // the user intended a spec, otherwise it's an unknown goal. | ||
| // TODO: This is probably not good logic, since the CLI behavior will change based on |
There was a problem hiding this comment.
This logic is preserved for compatibility, but we should probably consider changing it at some point.
| // which starts with a single dash, and we know is not a single dash flag). | ||
| if arg.starts_with("-") | ||
| || SPEC_RE.is_match(&arg) | ||
| || self.build_root.join(Path::new(&arg)).exists() |
There was a problem hiding this comment.
This is sync IO. Does sync vs. async matter at this point of the application?
There was a problem hiding this comment.
I don't think so - the Python equivalent was blocking on the same thing at the same point. And args are split long before we create a scheduler - we have to split them in order to get the option values for setting up the scheduler.
The only functionality in the legacy ArgSplitter not
replicated in Rust is distinguishing between
builtin/auxiliary goals and regular goals. This
can easily be done post-hoc, in Python, as
that distinction is not required in order to correctly
split args (now that we don't assign flags to
scopes in the ArgSplitter).
The tests are ported directly from the relevant
Python ones.
A future PR will actually wire this new ArgSplitter
up instead of the Python one.