Correctly parse arguments from environment variables #14376
Conversation
rustbot
added
A-configuration
GuillaumeGomez
force-pushed
the
parse-args-from-env-var
branch
from
a1db1f6 to
ed6a168
Compare
| output.extend( | ||
| env_val | ||
| .split_whitespace() | ||
| Shlex::new(env_val) | ||
| .into_iter() | ||
| .map(|s| (s.to_string(), def.clone())), |
There was a problem hiding this comment.
We'll need a test for this.
Ideally, the PR gets split up into two commits
- The first adds the test, showing the bad behavior
- This commit updates the test showing the good behavior
As discussed at https://doc.crates.io/contrib/process/working-on-cargo.html#submitting-a-pull-request
There was a problem hiding this comment.
Noted! Rewriting git history and using this occasion to add a test.
|
I'm unsure what the compatibility guarantees are around this. @ehuss if you have any insight, it'd be appreciated. In general, we recommend starting with an issue. That way we make sure we are all on the same page (e.g. the definition of the problem in this PR is unclear) and, if we go through multiple possible PRs for the problem (which is common enough), the conversation on the problem and solution space is centralized rather than split among all of the PRs. |
GuillaumeGomez
force-pushed
the
parse-args-from-env-var
branch
from
ed6a168 to
4b152ec
Compare
|
I opened #14381 too. |
| let mut expected = Vec::new(); | ||
| for sub in *value { | ||
| expected.push(( | ||
| sub.replace("\"", ""), |
There was a problem hiding this comment.
shlex removes (double) quotes. Not sure whether or not it's a good thing. If it's not the expected behaviour, I can implement a simple shell parser which keeps them.
There was a problem hiding this comment.
Blocked on discussion in #14381
Fixes #14381.
The current parsing doesn't correctly handle arguments like
--x="y z". To be noted: if you don't want the dependency, I can write a parser by hand too, shouldn't take many lines of code to do.r? @epage