Skip to content
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

uvx/uv tool run: Add context message before listing available tools when no arguments are provided #7641

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

kakkoyun
Copy link
Contributor

@kakkoyun kakkoyun commented Sep 23, 2024

Summary

Adds a helpful context message when uvx is run without arguments
To clarify, it is displaying the installed tools.

This addresses confusion, such as the one highlighted in issue #7348,
by making the output more user-friendly and informative.

Related #4024

Test Plan

Updated the test snapshots to include the new output.
Running the tests locally with cargo nextest run confirms that the tests pass.
The CI pipeline should also pass.

Manuel Testing

uvx

# Make sure you have the updated version of uv installed on your path.
# cargo install --path ./crates/uv --force
❯ uvx
Provide a command to invoke with `uvx <command>` or `uvx --from <package> <command>`.

The following tools are already installed:

black v24.8.0
- black
- blackd
ruff v0.6.7
- ruff

See `uvx --help` for more information.

uv tool list

# Make sure you have the updated version of uv installed on your path.
# cargo install --path ./crates/uv --force
❯ uv tool list
black v24.8.0
- black
- blackd
ruff v0.6.7
- ruff

uv tool run

# Make sure you have the updated version of uv installed on your path.
# cargo install --path ./crates/uv --force
❯ uv tool run
Provide a command to invoke with `uv tool run <command>` or `uv tool run --from <package> <command>`.

The following tools are already installed:

black v24.8.0
- black
- blackd
ruff v0.6.7
- ruff

See `uv tool run --help` for more information.

Signed-off-by: Kemal Akkoyun kakkoyun@gmail.com

@kakkoyun kakkoyun requested a review from zanieb September 23, 2024 14:25
@zanieb
Copy link
Member

zanieb commented Sep 23, 2024

I think you can be a bit more verbose here, like:

Provide a tool to invoke with `uvx <name>`. The following tools are already installed:

ruff v0.1.0:
- ruff
black v0.5.0:
- black
- blackd

There's a bit of awkwardness in the formatting of the list following the :. I usually expect something like:

This is a list of things:
- a
- b
- c

This deviates a bit from the initial scope of this change, but trying to construct a prompt that puts the list in context reveals a problem with the list itself. uvx takes the package as a target not the executable so some of the things we're listing here aren't actually valid arguments. For example, for blackd:


❯ uv tool install black
Resolved 6 packages in 259ms
Installed 6 packages in 7ms
 + black==24.8.0
 + click==8.1.7
 + mypy-extensions==1.0.0
 + packaging==24.1
 + pathspec==0.12.1
 + platformdirs==4.3.6
Installed 2 executables: black, blackd

❯ uvx blackd
  × No solution found when resolving tool dependencies:
  ╰─▶ Because blackd was not found in the package registry and you require blackd, we can conclude that your requirements are unsatisfiable.                                           

You actually need uvx --from black blackd to use blackd. I'm left wondering how we can convey this to the user in this listing?

For example, should we omit the executables:

Provide a tool to invoke with `uvx <name>`. The following tools are already installed:
- ruff 
- black

Should we just suggest using uv tool list to see already installed tools? etc.

@kakkoyun
Copy link
Contributor Author

@zanieb Thanks for the feedback. I will have another iteration.

@kakkoyun
Copy link
Contributor Author

I also like your suggestion on omitting the details on the bare run and referring to the uv tool list for additional information.

Let me see what I can do.

@zanieb
Copy link
Member

zanieb commented Sep 23, 2024

Another solution would be to change the behavior itself, e.g., allow uvx blackd to invoke the installed blackd from black without opt-in — then the current output makes sense. I don't really want to burden you with that user-experience concern since it requires a lot of context on how uv run and uv tool run work today, but I figure it's worth pointing out while we're here. I think we can improve the current message regardless and revisit possible changes to behavior later / separately. cc @charliermarsh

@kakkoyun
Copy link
Contributor Author

@zanieb I made the context message more descriptive. As a follow-up to this PR, I can create another to improve the error message when the user searches for an executable and fails.

Let me know what you think.

@zanieb
Copy link
Member

zanieb commented Sep 23, 2024

So, taking a deeper look at this... we're getting closer to just showing something like what Clap would show. For example, if we just make the command required Clap will show the help menu:

diff --git a/crates/uv-cli/src/lib.rs b/crates/uv-cli/src/lib.rs
index 84614fab3..913df97b8 100644
--- a/crates/uv-cli/src/lib.rs
+++ b/crates/uv-cli/src/lib.rs
@@ -3205,7 +3205,7 @@ pub struct ToolRunArgs {
     ///
     /// WARNING: The documentation for [`Self::command`] is not included in help output
     #[command(subcommand)]
-    pub command: Option<ExternalCommand>,
+    pub command: ExternalCommand,
 
     /// Use the given package to provide the command.
     ///
diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs
index 1a120f633..3ba74c656 100644
--- a/crates/uv/src/commands/tool/run.rs
+++ b/crates/uv/src/commands/tool/run.rs
@@ -63,7 +63,7 @@ impl Display for ToolRunCommand {
 
 /// Run a command.
 pub(crate) async fn run(
-    command: Option<ExternalCommand>,
+    command: ExternalCommand,
     from: Option<String>,
     with: &[RequirementsSource],
     show_resolution: bool,
@@ -80,9 +80,9 @@ pub(crate) async fn run(
     printer: Printer,
 ) -> anyhow::Result<ExitStatus> {
     // treat empty command as `uv tool list`
-    let Some(command) = command else {
-        return tool_list(false, false, &cache, printer).await;
-    };
+    // let Some(command) = command else {
+    //     return tool_list(false, false, &cache, printer).await;
+    // };
 
     let (target, args) = command.split();
     let Some(target) = target else {
diff --git a/crates/uv/src/settings.rs b/crates/uv/src/settings.rs
index 641e780a7..60fcd5600 100644
--- a/crates/uv/src/settings.rs
+++ b/crates/uv/src/settings.rs
@@ -287,7 +287,7 @@ impl RunSettings {
 #[allow(clippy::struct_excessive_bools)]
 #[derive(Debug, Clone)]
 pub(crate) struct ToolRunSettings {
-    pub(crate) command: Option<ExternalCommand>,
+    pub(crate) command: ExternalCommand,
     pub(crate) from: Option<String>,
     pub(crate) with: Vec<String>,
     pub(crate) with_editable: Vec<String>,
❯ cargo run -q -- tool run

Run a command provided by a Python package

Usage: uv tool run [OPTIONS] <COMMAND>

Options:
   ...

I think we actually do lose something by not listing the installed tools there. So I do think there's something to be said about having a message here, but we need to balance the content — e.g., we can suggest the help menu for more details.

I think I'd do something like:

Provide a command to invoke with `uvx <command>` or `uvx --from <package> <command>`.

The following tools are already installed:

black v24.8.0
- black
- blackd

See `uvx --help` for more information.

I'd consider indenting or changing the formatting of the list output but that will require some sort of flag for the tool_list function and I'd rather avoid that here and just keep it simple.

As another note, you'll need to customize the output based on the ToolRunCommand variant (i.e. uvx or uv tool run).

@zanieb zanieb added the error messages Messaging when something goes wrong label Sep 23, 2024
@kakkoyun kakkoyun force-pushed the add_info_message_for_empty_uvx_run branch from dcf1254 to c445f6e Compare September 24, 2024 09:57
@kakkoyun
Copy link
Contributor Author

@zanieb I agree that we should use more than just the same functionality provided by Clap. The users could have already used the help command or the --help flag to explain the functionality. Suggesting the existing help commands makes perfect sense. I will make sure to keep it concise.

@kakkoyun kakkoyun force-pushed the add_info_message_for_empty_uvx_run branch from c445f6e to a0cc8ad Compare September 24, 2024 11:43
@kakkoyun kakkoyun force-pushed the add_info_message_for_empty_uvx_run branch from b076501 to 68b116d Compare September 24, 2024 14:23
@kakkoyun kakkoyun requested a review from zanieb September 24, 2024 14:26
@kakkoyun kakkoyun force-pushed the add_info_message_for_empty_uvx_run branch 2 times, most recently from cc7c93b to 158ccfe Compare September 24, 2024 14:36
Comment on lines 122 to 124
No tools installed.

See `uv tool install --help` for more information.
Copy link
Member

Choose a reason for hiding this comment

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

I think these could be displayed on the same line. Should we be changing this in this pull request though? It seems like a separate change.

Similarly, we'll need to discuss if we should say "No tools installed" when there are malformed tools installed.

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 probably move all the changes to uv tool list behavior out of this and into a separate pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similarly, we'll need to discuss if we should say "No tools installed" when there are malformed tools installed.

Shall we display a different message? Basically, I want to avoid the case where all the tools are malformed and say here are the installed tools, but they are empty.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see. Now I understand why you're tweaking uv tool list. We probably don't want to see information about malformed tools in the uv tool run help at all. This sort of suggests we need to stop calling the uv tool list implementation directly? I think generally that will make things easier, i.e. if there are no installed tools the uv tool run implementation can handle that directly instead.

Copy link
Member

Choose a reason for hiding this comment

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

(this probably requires some refactor to uv tool list so we can avoid duplicating most of what it does?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this makes sense now. Let me try to refactor the code a little.

@kakkoyun kakkoyun force-pushed the add_info_message_for_empty_uvx_run branch 3 times, most recently from 50b7917 to 903c77a Compare September 24, 2024 19:54
@kakkoyun
Copy link
Contributor Author

@zanieb I hope that with the 903c77a, I finally captured what you have in mind 😅 For some reason, I was under the impression that we want to keep the behavior in sync with uv tool list. Now that I think about it, it's because the code reuse was already in place.

@kakkoyun kakkoyun changed the title Add context message before listing available tools when no arguments are provided uvx/uv tool run: Add context message before listing available tools when no arguments are provided Sep 25, 2024
@kakkoyun kakkoyun requested a review from zanieb September 25, 2024 19:55
…are provided

Adds a helpful context message when `uvx` is run without arguments
to clarify that it is displaying the installed tools.

This addresses confusion, such as the one highlighted in issue astral-sh#7348,
by making the output more user-friendly and informative.

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun kakkoyun force-pushed the add_info_message_for_empty_uvx_run branch 2 times, most recently from d105482 to 2e230ec Compare September 26, 2024 13:48
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@zanieb zanieb merged commit 3ce3403 into astral-sh:main Sep 26, 2024
61 checks passed
@kakkoyun kakkoyun deleted the add_info_message_for_empty_uvx_run branch September 27, 2024 09:44
@kakkoyun
Copy link
Contributor Author

charliermarsh pushed a commit that referenced this pull request Sep 27, 2024
Follow-up to #7641 with some minor changes to the implementation and a
simplification of the output
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Oct 7, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.4.15` -> `0.4.18` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>astral-sh/uv (astral-sh/uv)</summary>

### [`v0.4.18`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0418)

[Compare Source](astral-sh/uv@0.4.17...0.4.18)

##### Enhancements

-   Allow multiple source entries for each package in `tool.uv.sources` ([#&#8203;7745](astral-sh/uv#7745))
-   Add `.gitignore` file to `uv build` output directory ([#&#8203;7835](astral-sh/uv#7835))
-   Disable jemalloc on FreeBSD ([#&#8203;7780](astral-sh/uv#7780))
-   Respect `PAGER` env var when paging in `uv help` command ([#&#8203;5511](astral-sh/uv#5511))
-   Support `uv run -m foo` to run a module ([#&#8203;7754](astral-sh/uv#7754))
-   Use a top-level output directory for `uv build` in workspaces ([#&#8203;7813](astral-sh/uv#7813))
-   Update `uv init --package` command to match project name ([#&#8203;7670](astral-sh/uv#7670))
-   Add a custom suggestion for `uv add dotenv` ([#&#8203;7799](astral-sh/uv#7799))
-   Add detailed errors for `tool.uv.sources` deserialization failures ([#&#8203;7823](astral-sh/uv#7823))
-   Improve error message copy for failed builds ([#&#8203;7849](astral-sh/uv#7849))
-   Use `serde-untagged` to improve some untagged enum error messages ([#&#8203;7822](astral-sh/uv#7822))
-   Use build failure hints for `dotenv` errors, rather than in `uv add` ([#&#8203;7825](astral-sh/uv#7825))

##### Configuration

-   Add `UV_NO_SYNC` environment variable ([#&#8203;7752](astral-sh/uv#7752))

##### Bug fixes

-   Accept `git+` prefix in `tool.uv.sources` ([#&#8203;7847](astral-sh/uv#7847))
-   Allow spaces in path requirements ([#&#8203;7767](astral-sh/uv#7767))
-   Avoid reusing cached downloaded binaries with `--no-binary` ([#&#8203;7772](astral-sh/uv#7772))
-   Correctly trims values during wheel WHEEL file parsing ([#&#8203;7770](astral-sh/uv#7770))
-   Fix `uv tree --invert` for platform dependencies ([#&#8203;7808](astral-sh/uv#7808))
-   Fix encoding mismatch between python child process and uv ([#&#8203;7757](astral-sh/uv#7757))
-   Reject self-dependencies in `uv add` ([#&#8203;7766](astral-sh/uv#7766))
-   Respect `tool.uv.environments` for legacy virtual workspace roots ([#&#8203;7824](astral-sh/uv#7824))
-   Retain empty extras on workspace members ([#&#8203;7762](astral-sh/uv#7762))
-   Use file stem when parsing cached wheel names ([#&#8203;7773](astral-sh/uv#7773))

##### Rust API

-   Make `FlatDistributions` public ([#&#8203;7833](astral-sh/uv#7833))

##### Documentation

-   Fix table of contents sizing ([#&#8203;7751](astral-sh/uv#7751))
-   GitLab Integration documentation ([#&#8203;6857](astral-sh/uv#6857))
-   Update documentation to setup-uv@v3 ([#&#8203;7807](astral-sh/uv#7807))
-   Use `uv publish` instead of twine in docs ([#&#8203;7837](astral-sh/uv#7837))
-   Fix typo in `projects.md` ([#&#8203;7784](astral-sh/uv#7784))

### [`v0.4.17`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0417)

[Compare Source](astral-sh/uv@0.4.16...0.4.17)

##### Enhancements

-   Add `uv build --all` to build all packages in a workspace ([#&#8203;7724](astral-sh/uv#7724))
-   Add support for `uv init --script` ([#&#8203;7565](astral-sh/uv#7565))
-   Add support for upgrading build environment for installed tools (`uv tool upgrade --python`) ([#&#8203;7605](astral-sh/uv#7605))
-   Initialize a Git repository in `uv init` ([#&#8203;5476](astral-sh/uv#5476))
-   Respect `--quiet` flag in `uv build` ([#&#8203;7674](astral-sh/uv#7674))
-   Add context message before listing available tools in `uvx` ([#&#8203;7641](astral-sh/uv#7641))

##### Bug fixes

-   Don't create Python bytecode files during interpreter discovery ([#&#8203;7707](astral-sh/uv#7707))
-   Escape glob patterns in workspace member discovery ([#&#8203;7709](astral-sh/uv#7709))
-   Avoid prefetching source distributions with unbounded lower-bound ranges ([#&#8203;7683](astral-sh/uv#7683))

##### Documentation

-   Add `uv build` and `uv publish` to features overview ([#&#8203;7716](astral-sh/uv#7716))
-   Add documentation on cache versioning ([#&#8203;7693](astral-sh/uv#7693))
-   Spell out the names of the Docker images for easier copy-paste ([#&#8203;7706](astral-sh/uv#7706))
-   Document uv-with-Jupyter workflows ([#&#8203;7625](astral-sh/uv#7625))
-   Note that `uv lock --upgrade-package` retains locked versions ([#&#8203;7694](astral-sh/uv#7694))

### [`v0.4.16`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0416)

[Compare Source](astral-sh/uv@0.4.15...0.4.16)

##### Enhancements

-   Add `uv publish` ([#&#8203;7475](astral-sh/uv#7475))
-   Add a `--project` argument to run a command from a project directory ([#&#8203;7603](astral-sh/uv#7603))
-   Display Python implementation when creating environments ([#&#8203;7652](astral-sh/uv#7652))
-   Implement trusted publishing for `uv publish` ([#&#8203;7548](astral-sh/uv#7548))
-   Respect lockfile preferences for `--with` requirements ([#&#8203;7627](astral-sh/uv#7627))
-   Unhide the `--directory` option ([#&#8203;7653](astral-sh/uv#7653))
-   Allow requesting free-threaded Python interpreters ([#&#8203;7431](astral-sh/uv#7431))
-   Show a dedicated PubGrub hint for `--unsafe-best-match` ([#&#8203;7645](astral-sh/uv#7645))
-   Add resolver error checking for conflicting distributions ([#&#8203;7595](astral-sh/uv#7595))

##### Bug fixes

-   Avoid adding double-newlines for CRLF ([#&#8203;7640](astral-sh/uv#7640))
-   Avoid retaining forks when `requires-python` range changes ([#&#8203;7624](astral-sh/uv#7624))
-   Determine if pre-release Python downloads should be allowed using the version specifiers ([#&#8203;7638](astral-sh/uv#7638))
-   Fix `link-mode=clone` for directories on Linux ([#&#8203;7620](astral-sh/uv#7620))
-   Improve Python executable name discovery when using alternative implementations ([#&#8203;7649](astral-sh/uv#7649))
-   Require opt-in to use alternative Python implementations ([#&#8203;7650](astral-sh/uv#7650))
-   Use the first pre-release discovered when only pre-release Python versions are available ([#&#8203;7666](astral-sh/uv#7666))

##### Documentation

-   Document environment variable that disables printing of virtual environment name in prompt ([#&#8203;7648](astral-sh/uv#7648))
-   Remove double whitespaces from the code ([#&#8203;7623](astral-sh/uv#7623))
-   Use anchorlinks rather than permalinks ([#&#8203;7626](astral-sh/uv#7626))

##### Preview features

-   Add build backend scaffolding ([#&#8203;7662](astral-sh/uv#7662))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error messages Messaging when something goes wrong
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants