Display the default value of `--link-mode` in help
Summary
Part of #6173 .
Test Plan
Inspect the help output for the following commands.
cargo run pip install --help
cargo run pip sync --help
cargo run pip venv --help
Based on the CI failure
running 1 test
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot: tool_install_version-3
Source: crates/uv/tests/tool_install.rs:306
────────────────────────────────────────────────────────────────────────────────
Expression: fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap()
────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬───────────────────────────────────────────────────────────────────
4 4 │ { name = "blackd", install-path = "[TEMP_DIR]/bin/blackd" },
5 5 │ ]
6 6 │
7 7 │ [tool.options]
8 │-exclude-newer = "2024-03-25T00:00:00Z"
8 │+exclude-newer = "2024-03-25T00:00:00Z"
9 │+link-mode = "hardlink"
────────────┴───────────────────────────────────────────────────────────────────
It seems the tool receipt will record the given options. Therefore, we should only set the value when it differs from the default.
Also note that the default value varies by platform. We might need some help text about this to prevent confusion when reading the cli reference on the site.
I'm not sure how we should handle the snapshot of cli references, as the output depends on the platform it's generated on. :sweat_smile:
Lots of complications to consider here! I'm glad you just started with one option. We're going to be focused on 0.3.0 for a bit here — but after that I can try to help with the CLI reference guide and the snapshots. @charliermarsh is also going to need to take a look regarding the .then_some pattern, I'm not sure what implications this has for our tracking of settings in general.
Or perhaps for this kind of platform-dependent default value, it would be better to display something similar to the CLI reference? e.g.
--link-mode <LINK_MODE> The method to use when installing packages from the global cache. Defaults to `clone` (also known as Copy-on-Write) on macOS, and `hardlink` on Linux and
Windows [env: UV_LINK_MODE=] [possible values: clone, copy, hardlink, symlink]
I think it's okay to show the default value per-platform in the CLI help menu, but yeah we'll want show both in the reference documentation — sounds complicated. Might be easier to always show them both for the sake of snapshots — but hopefully it's only a small amount of snapshots that actually include the help menu and we can just use filters...
Based on the CI feedback, resetting only for ToolOptions seems more correct. Now, it only fails on cli references :)
I also noticed that we could implement the Display for LinkMode to allow setting the default value via default_value = LinkMode::default().to_string(). This way, we wouldn't need to change the type. Both approaches would set the value to the default if it's not provided.
Might be easier to always show them both for the sake of snapshots — but hopefully it's only a small amount of snapshots that actually include the help menu and we can just use filters...
Or we could add it (and other options whose doc already shows more descriptive information about the default value) to a group like doc-hide-default, which indicates that we don't want this to be displayed in the cli reference and allows us to omit this default value during generation.
As more reference, I had to undo the default value in https://github.com/astral-sh/uv/pull/6835
We don't have access to ArgMatches, though maybe we could refactor things so we do.
As more reference, I had to undo the default value in #6835
We don't have access to
ArgMatches, though maybe we could refactor things so we do.
I guess the following should work:
diff --git a/crates/uv/src/lib.rs b/crates/uv/src/lib.rs
index 080ed988..c5426578 100644
--- a/crates/uv/src/lib.rs
+++ b/crates/uv/src/lib.rs
@@ -1265,0 +1267,15 @@ async fn run_project(
+fn get_given_args(matches: &clap::ArgMatches) -> std::collections::HashSet<&clap::Id> {
+ let mut args = matches;
+ while let Some((_sub_cmd, sub_args)) = args.subcommand() {
+ args = sub_args;
+ }
+ args.ids()
+ .filter(|id| {
+ !matches!(
+ args.value_source(id.as_str()),
+ Some(clap::parser::ValueSource::DefaultValue)
+ )
+ })
+ .collect()
+}
+
@@ -1278,3 +1294,6 @@ where
- // `std::env::args` is not `Send` so we parse before passing to our runtime
- // https://github.com/rust-lang/rust/pull/48005
- let cli = match Cli::try_parse_from(args) {
+ let cmd = Cli::command();
+ let matches = cmd.get_matches_from(args);
+ let cli = match clap::FromArgMatches::from_arg_matches(&matches) {
+ // `std::env::args` is not `Send` so we parse before passing to our runtime
+ // https://github.com/rust-lang/rust/pull/48005
+ // let cli = match Cli::try_parse_from(args) {
This should not alter the way uv currently parses arguments but provide us with the ability to retrieve given arguments from ArgMatches.
I think we may just need to keep this as documentation and not as a structured Clap default. As-is (in this PR) it's incorrect in the generated reference too, since it's dynamic.
I'm going to close for now but let me know if you want to follow-up in some way. Thank you for your work here.