Tweaks to the uvx help menu when no command is provided#7740
Tweaks to the uvx help menu when no command is provided#7740charliermarsh merged 1 commit intomainfrom
uvx help menu when no command is provided#7740Conversation
| match list_available_tools(invocation_source, &cache, printer).await { | ||
| // It is a failure because user misses a required tool name. | ||
| Ok(()) => return Ok(ExitStatus::Error), | ||
| Err(err) => return Err(err), |
There was a problem hiding this comment.
Handling this case with ? instead.
There was a problem hiding this comment.
Definitely, way cleaner 🙈
| let no_tools_installed_msg = | ||
| "No tools installed. See `uv tool install --help` for more information."; |
There was a problem hiding this comment.
After playing with this a bit more, I was worried users would feel like they had to use uv tool install before they could use uv tool run which isn't the case so I dropped this message.
| }; | ||
|
|
||
| let mut tools = installed_tools.tools()?.into_iter().collect::<Vec<_>>(); | ||
| tools.sort_by_key(|(name, _)| name.clone()); |
There was a problem hiding this comment.
Instead of using sort_by_key I use sorted_by to avoid the extra clone.
| // Skip invalid tools | ||
| .filter_map(|(name, tool)| { | ||
| tool.ok().and_then(|_| { | ||
| installed_tools | ||
| .version(&name, cache) | ||
| .ok() | ||
| .map(|version| (name, version)) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
I do all the filtering here, so we don't need to use a buf: String later.
|
|
||
| let mut buf = String::new(); | ||
| for (name, tool) in tools { | ||
| // Skip invalid tools. | ||
| let Ok(tool) = tool else { | ||
| continue; | ||
| }; | ||
|
|
||
| // Output tool name and version. | ||
| let Ok(version) = installed_tools.version(&name, cache) else { | ||
| continue; | ||
| }; | ||
| writeln!(buf, "{}", format!("{name} v{version}").bold())?; | ||
|
|
||
| // Output tool entrypoints. | ||
| for entrypoint in tool.entrypoints() { | ||
| writeln!(buf, "- {}", entrypoint.name)?; | ||
| } | ||
| // Display the tools | ||
| writeln!(printer.stdout(), "The following tools are installed:\n")?; | ||
| for (name, version) in tools { | ||
| writeln!( | ||
| printer.stdout(), | ||
| "- {} v{version}", | ||
| format!("{name}").bold() | ||
| )?; | ||
| } |
There was a problem hiding this comment.
I dropped the entrypoints, mostly to simplify the output and avoid confusing around --from usage which is a bit more advanced and hard to spell here.
| The following tools are installed: | ||
|
|
||
| black v24.2.0 | ||
| - black | ||
| - blackd | ||
| - black v24.2.0 |
There was a problem hiding this comment.
I went back and forth on a newline here. I think it looks better if we have an extra newline separating the list? Relevant for #7687
There was a problem hiding this comment.
The extra line looks way cleaner to me. If you're okay with me putting it here, I will change the other PR.
There was a problem hiding this comment.
👍 yeah sorry I think I (wrongly) suggested you remove it over there
kakkoyun
left a comment
kakkoyun
left a comment
There was a problem hiding this comment.
LGTM. Thanks for cleaning things up and making the message much clearer.
These changes made me realize that I still write Rust as if I’m writing Go 😬
| match list_available_tools(invocation_source, &cache, printer).await { | ||
| // It is a failure because user misses a required tool name. | ||
| Ok(()) => return Ok(ExitStatus::Error), | ||
| Err(err) => return Err(err), |
There was a problem hiding this comment.
Definitely, way cleaner 🙈
| cache: &Cache, | ||
| printer: Printer, | ||
| ) -> anyhow::Result<()> { | ||
| let help = format!( |
| let no_tools_installed_msg = | ||
| "No tools installed. See `uv tool install --help` for more information."; |
3 participants
Follow-up to #7641 with some minor changes to the implementation and a simplification of the output