Improve handling of command arguments in uv run and uv tool run
#4404
Conversation
| #[derive(Parser, Debug, Clone)] | ||
| pub(crate) enum ExternalCommand { | ||
| #[command(external_subcommand)] | ||
| Cmd(Vec<OsString>), | ||
| } |
crates/uv/src/cli.rs
Outdated
| impl ExternalCommand { | ||
| pub(crate) fn split(&self) -> (Option<&OsString>, &[OsString]) { | ||
| match self.deref().as_slice() { | ||
| [] => (None, &[]), | ||
| [cmd, args @ ..] => (Some(cmd), args), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Makes life easier in the implementations, we special case the first argument.
| mod common; | ||
|
|
||
| #[test] | ||
| fn tool_run_args() { |
There was a problem hiding this comment.
Our first tool run tests!
| @@ -44,7 +44,7 @@ pub const INSTA_FILTERS: &[(&str, &str)] = &[ | |||
| (r"uv.exe", "uv"), | |||
| // uv version display | |||
| ( | |||
| r"uv \d+\.\d+\.\d+( \(.*\))?", | |||
| r"uv(-.*)? \d+\.\d+\.\d+( \(.*\))?", | |||
There was a problem hiding this comment.
We output uv-run ... for uv run --version. Kind of annoying tbh but it's Clap's behavior I think so just going to filter it.
| target: Option<String>, | ||
| mut args: Vec<OsString>, | ||
| command: ExternalCommand, |
There was a problem hiding this comment.
We split this into target and args late so we can avoid borrowing, cloning, or otherwise painfully popping the first item from the vector.
crates/uv/src/cli.rs
Outdated
|
|
||
| impl ExternalCommand { | ||
| pub(crate) fn split(&self) -> (Option<&OsString>, &[OsString]) { | ||
| match self.deref().as_slice() { |
There was a problem hiding this comment.
It is odd to need to use .deref() explicitly. Does just self.as_slice() work?
There was a problem hiding this comment.
Probably just me being dumb, thanks!
Honestly I'm not sure about the details, just that this is the standard flag for this purpose. My understanding is as follows though:
|
No reason for these to be special-cased and I need them for #4404 tests
| pub(crate) fn split(&self) -> (Option<&OsString>, &[OsString]) { | ||
| match self.as_slice() { | ||
| [] => (None, &[]), | ||
| [cmd, args @ ..] => (Some(cmd), args), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
That's implemented in std as split_first
| .collect::<Vec<_>>(); | ||
| let (target, args) = command.split(); | ||
| let Some(target) = target else { | ||
| return Err(anyhow::anyhow!("No tool command provided")); |
| let Some(target) = target.to_str() else { | ||
| return Err(anyhow::anyhow!("Tool command could not be parsed as UTF-8 string. Use `--from` to specify the package name.")); | ||
| }; |
There was a problem hiding this comment.
This can be .context("..."), anyhow conveniently implements it on option too
| .arg("--cache-dir") | ||
| .arg(self.cache_dir.path()) | ||
| .env("VIRTUAL_ENV", self.venv.as_os_str()) | ||
| .env("UV_NO_WRAP", "1") | ||
| .env("UV_TOOLCHAIN_DIR", "") | ||
| .env("UV_TEST_PYTHON_PATH", &self.python_path()) | ||
| .current_dir(&self.temp_dir); | ||
|
|
||
| if cfg!(all(windows, debug_assertions)) { | ||
| // TODO(konstin): Reduce stack usage in debug mode enough that the tests pass with the | ||
| // default windows stack of 1MB | ||
| command.env("UV_STACK_SIZE", (4 * 1024 * 1024).to_string()); | ||
| } |
There was a problem hiding this comment.
We probably need to abstract those away soon, we're having them in a lot of subcommand test instantiators now
Closes #4390
We no longer require
--to disambiguate child command options that overlap with uv options.