Skip to content

Add --show-with to uv tool list to list packages included by --with #13264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
May 6, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4334,6 +4334,10 @@ pub struct ToolListArgs {
#[arg(long)]
pub show_version_specifiers: bool,

/// Whether to display the extra requirements(s) installed with each tool.
#[arg(long)]
pub show_with: bool,

// Hide unused global Python options.
#[arg(long, hide = true)]
pub python_preference: Option<PythonPreference>,
Expand Down
28 changes: 26 additions & 2 deletions crates/uv/src/commands/tool/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::printer::Printer;
pub(crate) async fn list(
show_paths: bool,
show_version_specifiers: bool,
show_with: bool,
cache: &Cache,
printer: Printer,
) -> Result<ExitStatus> {
Expand Down Expand Up @@ -80,18 +81,41 @@ pub(crate) async fn list(
String::new()
};

let extra_requirements = if show_with {
let extra_requirements = tool
.requirements()
.iter()
.filter(|req| req.name != name)
.map(|req| {
if !req.source.to_string().is_empty() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the requirement source display ever empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just copied the filtering from the code block above, but removing any if them doesn't change the output or the status of the tests? So I guess either remove them, or if the are required ideally add test cases to capture that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's possible for it to be empty from looking at the implementation 🤷‍♀️ not sure why it's that way.

format!("{}{}", req.name, req.source)
} else {
req.name.to_string()
}
})
.filter(|s| !s.is_empty())
.join(", ");
if extra_requirements.is_empty() {
String::new()
} else {
format!(" [with: {extra_requirements}]")
}
} else {
String::new()
};
Copy link
Member

@zanieb zanieb May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could rewrite this entire block as something like

        let extra_requirements = (show_with && !tool.requirements().is_empty())
            .then(|| {
                let requirements = tool
                    .requirements()
                    .iter()
                    // Exclude the package itself
                    .filter(|req| req.name != name)
                    .map(|req| format!("{}{}", req.name, req.source))
                    .join(", ");
                format!(" [with: {requirements}]")
            })
            .unwrap_or_default();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay that broke some stuff, need to add a check that requirements exists, I guess since installing without any --with requirements still returns the original package, and hence returns an empty [width: ] string. Will add the original test without any --with requirements which I was supposed to test this back in the test suite.

Copy link
Contributor Author

@gaardhus gaardhus May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm super new to rust, so sorry if there is something I don't ace.

Since the requirements array may be empty, I think we do need an if else clause here before inserting the values?

So currently working implementation would be

        let with_requirements = (show_with && !tool.requirements().is_empty())
            .then(|| {
                let requirements = tool
                    .requirements()
                    .iter()
                    .filter(|req| req.name != name)
                    .map(|req| format!("{}{}", req.name, req.source))
                    .join(", ");
                if requirements.is_empty() {
                    String::new()
                } else {
                    format!(" [with: {requirements}]")
                }
            })
            .unwrap_or_default();

My initial implementation was based on the version_specifier parsing on the lines above. So if you prefer unwrap_or_default should I also change that chunk to mirror that, ie.

        let version_specifier = show_version_specifiers
            .then(|| {
                let specifiers = tool
                    .requirements()
                    .iter()
                    .filter(|req| req.name == name)
                    .map(|req| req.source.to_string())
                    .filter(|s| !s.is_empty())
                    .join(", ");
                if specifiers.is_empty() {
                    String::new()
                } else {
                    format!(" [required: {specifiers}]")
                }
            })
            .unwrap_or_default();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just do (show_with && tool.requirements().len() > 1), though it's a bit less elegant since it doesn't capture that after the filter.

The prettier way is like..

        let extra_requirements = show_with
            .then(|| {
                tool.requirements()
                    .iter()
                    // Exclude the package itself
                    .filter(|req| req.name != name)
                    .peekable()
            })
            // Omit the `[with: ...]` if there are no extra requirements
            .filter(|requirements| requirements.peek().is_some())
            .map(|requirements| {
                let requirements = requirements
                    .iter()
                    .map(|req| format!("{}{}", req.name, req.source))
                    .join(", ");
                format!(" [with: {requirements}]")
            })
            .unwrap_or_default();

Copy link
Member

@zanieb zanieb May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just do (show_with && tool.requirements().len() > 1), though it's a bit less elegant since overlaps with the logic encoded in the filter.

The prettier way is like..

        let extra_requirements = show_with
            .then(|| {
                tool.requirements()
                    .iter()
                    // Exclude the package itself
                    .filter(|req| req.name != name)
                    .peekable()
            })
            // Omit the `[with: ...]` if there are no extra requirements
            .filter(|requirements| requirements.peek().is_some())
            .map(|requirements| {
                let requirements = requirements
                    .iter()
                    .map(|req| format!("{}{}", req.name, req.source))
                    .join(", ");
                format!(" [with: {requirements}]")
            })
            .unwrap_or_default();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your suggestions and detailed review!

I couldn't make the peekable work, the compiler kept throwing errors about mutability.

So I tried reimplementing and unifying the two functions like this

        let tool_requirements = (show_version_specifiers | show_with)
            .then(|| tool.requirements())
            .unwrap_or_default();

        let version_specifier = show_version_specifiers
            .then(|| {
                let specifiers = tool_requirements
                    .iter()
                    .filter(|req| req.name == name)
                    .map(|req| req.source.to_string())
                    .filter(|s| !s.is_empty())
                    .join(", ");
                (!specifiers.is_empty())
                    .then(|| format!(" [required: {specifiers}]"))
                    .unwrap_or_default()
            })
            .unwrap_or_default();

        let with_requirements = (show_with && tool_requirements.len() > 1)
            .then(|| {
                let requirements = tool_requirements
                    .iter()
                    .filter(|req| req.name != name)
                    .map(|req| format!("{}{}", req.name, req.source))
                    .join(", ");
                format!(" [with: {requirements}]")
            })
            .unwrap_or_default();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry about that, the mutable reference thing is annoying... I'm not sure if there's a fix for that (but I'll ask someone who knows more than me).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah .take_if(|requirements| requirements.peek().is_some()) works instead of .filter(...)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Rust's idioms around options take a while to learn, but they are nice once you get the hang of it)

Copy link
Contributor Author

@gaardhus gaardhus May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then they become

        let version_specifier = show_version_specifiers
            .then(|| {
                tool.requirements()
                    .iter()
                    .filter(|req| req.name == name)
                    .map(|req| req.source.to_string())
                    .filter(|s| !s.is_empty())
                    .peekable()
            })
            .take_if(|specifiers| specifiers.peek().is_some())
            .map(|mut specifiers| {
                let specifiers = specifiers.join(", ");
                format!(" [required: {specifiers}]")
            })
            .unwrap_or_default();

        let with_requirements = show_with
            .then(|| {
                tool.requirements()
                    .iter()
                    .filter(|req| req.name != name)
                    .peekable()
            })
            .take_if(|requirements| requirements.peek().is_some())
            .map(|requirements| {
                let requirements = requirements
                    .map(|req| format!("{}{}", req.name, req.source))
                    .join(", ");
                format!(" [with: {requirements}]")
            })
            .unwrap_or_default();

if I'm not mistaken?


if show_paths {
writeln!(
printer.stdout(),
"{} ({})",
format!("{name} v{version}{version_specifier}").bold(),
format!("{name} v{version}{version_specifier}{extra_requirements}").bold(),
installed_tools.tool_dir(&name).simplified_display().cyan(),
)?;
} else {
writeln!(
printer.stdout(),
"{}",
format!("{name} v{version}{version_specifier}").bold()
format!("{name} v{version}{version_specifier}{extra_requirements}").bold()
)?;
}

Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,7 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
commands::tool_list(
args.show_paths,
args.show_version_specifiers,
args.show_with,
&cache,
printer,
)
Expand Down
3 changes: 3 additions & 0 deletions crates/uv/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,7 @@ impl ToolUpgradeSettings {
pub(crate) struct ToolListSettings {
pub(crate) show_paths: bool,
pub(crate) show_version_specifiers: bool,
pub(crate) show_with: bool,
}

impl ToolListSettings {
Expand All @@ -781,13 +782,15 @@ impl ToolListSettings {
let ToolListArgs {
show_paths,
show_version_specifiers,
show_with,
python_preference: _,
no_python_downloads: _,
} = args;

Self {
show_paths,
show_version_specifiers,
show_with,
}
}
}
Expand Down
96 changes: 96 additions & 0 deletions crates/uv/tests/it/tool_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,99 @@ fn tool_list_show_version_specifiers() {
----- stderr -----
"###);
}

#[test]
fn tool_list_show_with() {
let context = TestContext::new("3.12").with_filtered_exe_suffix();
let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");

// Install `flask` with extra requirements
context
.tool_install()
.arg("flask")
.arg("--with")
.arg("requests")
.arg("--with")
.arg("black==24.2.0")
.env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str())
.env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str())
.assert()
.success();

// Install `black` without extra requirements
context
.tool_install()
.arg("black==24.2.0")
.env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str())
.env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str())
.assert()
.success();

// Test with --show-with
uv_snapshot!(context.filters(), context.tool_list().arg("--show-with")
.env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str())
.env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
black v24.2.0
- black
- blackd
flask v3.0.2 [with: requests, black==24.2.0]
- flask

----- stderr -----
"###);

// Test with both --show-with and --show-paths
uv_snapshot!(context.filters(), context.tool_list().arg("--show-with").arg("--show-paths")
.env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str())
.env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
black v24.2.0 ([TEMP_DIR]/tools/black)
- black ([TEMP_DIR]/bin/black)
- blackd ([TEMP_DIR]/bin/blackd)
flask v3.0.2 [with: requests, black==24.2.0] ([TEMP_DIR]/tools/flask)
- flask ([TEMP_DIR]/bin/flask)

----- stderr -----
"###);

// Test with both --show-with and --show-version-specifiers
uv_snapshot!(context.filters(), context.tool_list().arg("--show-with").arg("--show-version-specifiers")
.env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str())
.env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
black v24.2.0 [required: ==24.2.0]
- black
- blackd
flask v3.0.2 [with: requests, black==24.2.0]
- flask

----- stderr -----
"###);

// Test with all flags
uv_snapshot!(context.filters(), context.tool_list()
.arg("--show-with")
.arg("--show-version-specifiers")
.arg("--show-paths")
.env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str())
.env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
black v24.2.0 [required: ==24.2.0] ([TEMP_DIR]/tools/black)
- black ([TEMP_DIR]/bin/black)
- blackd ([TEMP_DIR]/bin/blackd)
flask v3.0.2 [with: requests, black==24.2.0] ([TEMP_DIR]/tools/flask)
- flask ([TEMP_DIR]/bin/flask)

----- stderr -----
"###);
}
2 changes: 2 additions & 0 deletions docs/reference/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -4176,6 +4176,8 @@ uv tool list [OPTIONS]

</dd><dt id="uv-tool-list--show-version-specifiers"><a href="#uv-tool-list--show-version-specifiers"><code>--show-version-specifiers</code></a></dt><dd><p>Whether to display the version specifier(s) used to install each tool</p>

</dd><dt id="uv-tool-list--show-with"><a href="#uv-tool-list--show-with"><code>--show-with</code></a></dt><dd><p>Whether to display the extra requirements(s) installed with each tool</p>

</dd><dt id="uv-tool-list--verbose"><a href="#uv-tool-list--verbose"><code>--verbose</code></a>, <code>-v</code></dt><dd><p>Use verbose output.</p>

<p>You can configure fine-grained logging using the <code>RUST_LOG</code> environment variable. (&lt;https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html#directives&gt;)</p>
Expand Down
Loading