Add uv tool list --show-python to show python version#15312
Add uv tool list --show-python to show python version#15312pythonweb2 wants to merge 7 commits intoastral-sh:mainfrom pythonweb2:uv-tool-add-python-list-option
uv tool list --show-python to show python version#15312Conversation
|
We'll need a test case in |
|
I added tests @zanieb and made some changes in where the version gets pulled from. Lmk if it looks OK. |
| .unwrap_or_default(); | ||
|
|
||
| let python_version = if show_python { | ||
| match installed_tools.get_environment(&name, cache) { |
There was a problem hiding this comment.
Hm. I'm surprised we don't need to read the environment elsewhere here.
installed_tools.version(&name, cache) reads the environment. We probably don't want to do that multiple times.
I think might want to get the environment above and re-use it for both of these operations. That'll be more performant and we don't need to handle error cases multiple times.
I'd probably do something like...
- Add a new
ToolEnvironmenttype that's returned fromget_environment. It can just wrapPythonEnvironment. You can implement deref,into_inner, etc. (see other patterns like that) so it's not annoying to work with. There are only a few callsites so this shouldn't be bad. - Move the
InstalledTools::versionfunction toToolEnvironment. There are also only a couple call-sites to update here too. - Now in this function, get the environment once, then derive the tool and interpreter versions from it.
| match installed_tools.get_environment(&name, cache) { | ||
| Ok(Some(env)) => { | ||
| let interpreter = env.interpreter(); | ||
| format!(" [{}]", interpreter.markers().implementation_version()) |
There was a problem hiding this comment.
I'd just use interpreter.python_full_version(). I think we also probably want to include the implementation name. e.g., CPython 3.13.1.
There was a problem hiding this comment.
Alternatively we could display [python: <version>]. I don't know if I feel strongly. That'd be consistent with the other displays? but [python: <implementation> <version>] feels too verbose.
There was a problem hiding this comment.
Yeah I thought that [python: ... felt verbose as well. If the implementation is added, is seems like that would be self-explanatory enough.
| let interpreter = env.interpreter(); | ||
| format!(" [{}]", interpreter.markers().implementation_version()) | ||
| } | ||
| Ok(None) => String::from(" [python: not found]"), |
There was a problem hiding this comment.
We should skip display of the entire tool if the environment is missing. That should be resolve by the refactor I suggested.
| format!(" [{}]", interpreter.markers().implementation_version()) | ||
| } | ||
| Ok(None) => String::from(" [python: not found]"), | ||
| Err(e) => format!(" [python: error: {e}]"), |
There was a problem hiding this comment.
We'll want to display this as a warning as done for tool version handling — this should also be resolved by the refactor.
<!-- Thank you for contributing to uv! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> Closes astral-sh#15312 Closes astral-sh#16237 --------- Co-authored-by: pythonweb2 <[email protected]> Co-authored-by: Wade Roberts <[email protected]>
2 participants
Summary
Display information about the python version when using the
uv tool listcli command.Test Plan
Added a test that tests the option, and then also a test that tests all of the
--show-*options in the command.