-
Notifications
You must be signed in to change notification settings - Fork 413
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
Add support for listing system toolchains #4172
Conversation
b3f708c
to
943c963
Compare
2c3e3b6
to
26b90a6
Compare
630f39a
to
b3921b6
Compare
Example
|
Will probably add |
f0a6021
to
0c2856d
Compare
Still reviewing, but the first question in my head when I see the output is: why do some installations have a path and some don't? |
The ones that don't have a path aren't downloaded yet. Couldn't find a nice way to represent that yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Concerning the output, I think my confusion stems from a very strong a priori bias that the list I'm looking at reflects toolchains that are already installed. Maybe I'm an odd duck and most won't have that bias, and perhaps therefore most won't be confused. Not sure.
One idea is to add something like <available to install>
or something for each toolchain that is available but not installed. But... I could see that being noisy. And it's just kind of redundant info repeated a lot.
I think I still personally like listing only installed toolchains by default, and having an opt-in flag like --available
or whatever that shows the bigger output.
0c2856d
to
e75d514
Compare
Includes system interpreters in `uv toolchain list`. This includes a refactor of `find_toolchain` to support iterating over all toolchains that match a request rather than ending earlier.
e75d514
to
d893af2
Compare
Thanks for the review. I think you have some fair concerns about the output. Maybe I'm over-optimizing on the fact that we dynamically fetch requested interpreters so it doesn't really matter if something is installed or not because you can I added the |
Maybe we shouldn't show paths by default and just exclude system interpreters that overlap with managed interpreters by default. Then the output would be nice and simple like I want :) I will explore... probably as a follow-up. |
Totally in favor of this! If we're going to try something new, now is definitely the time.
👍 |
Includes system interpreters in
uv toolchain list
.This includes a refactor of
find_toolchain
to support iterating over all toolchainsthat match a request rather than ending earlier.