Update venv labeling to show [uv] when uv is available#810
Update venv labeling to show [uv] when uv is available#810
Conversation
Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com>
Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com>
eleanorjboyd
force-pushed
the
copilot/update-venv-label-for-uv
branch
from
559ecdf to
388eb78
Compare
| */ | ||
| export let VenvManagerStrings: typeof VenvManagerStringsNoUv | typeof VenvManagerStringsWithUv = VenvManagerStringsNoUv; | ||
|
|
||
| export function setVenvManagerStrings(val: typeof VenvManagerStringsNoUv | typeof VenvManagerStringsWithUv) { |
There was a problem hiding this comment.
@karthiknadig can you just review this to make sure this structure of setting strings makes sense and follows good coding practices
There was a problem hiding this comment.
This actually looks a bit weird. There are several string duplicates.
There was a problem hiding this comment.
@karthiknadig moved to duplicates to a diff function so there shouldn't be repeats. Lmk your thoughts
src/managers/builtin/venvManager.ts
Outdated
|
|
||
| readonly name: string; | ||
| readonly displayName: string; | ||
| displayName: string; // Made mutable to update with uv detection |
There was a problem hiding this comment.
Removed the comment as requested. The code is now cleaner without the explanatory comment. Commit: bb30e4f
src/managers/builtin/venvUtils.ts
Outdated
| const progressTitle = useUv | ||
| ? l10n.t( | ||
| 'Creating virtual environment named {0} using python version {1} [uv].', | ||
| path.basename(envPath), | ||
| basePython.version, | ||
| ) | ||
| : l10n.t( | ||
| 'Creating virtual environment named {0} using python version {1}.', | ||
| path.basename(envPath), | ||
| basePython.version, | ||
| ); |
There was a problem hiding this comment.
@copilot may be something like this, more compact?
| const progressTitle = useUv | |
| ? l10n.t( | |
| 'Creating virtual environment named {0} using python version {1} [uv].', | |
| path.basename(envPath), | |
| basePython.version, | |
| ) | |
| : l10n.t( | |
| 'Creating virtual environment named {0} using python version {1}.', | |
| path.basename(envPath), | |
| basePython.version, | |
| ); | |
| const progressTitle = l10n.t( | |
| 'Creating virtual environment named {0} using python version {1} {useUv? "[uv]": ""}.', | |
| path.basename(envPath), | |
| basePython.version, | |
| ); |
There was a problem hiding this comment.
Implemented your suggestion for a more compact approach using a single l10n.t() call with conditional text. This eliminates the duplicate string and makes the code cleaner. Commit: bb30e4f
…ssage Co-authored-by: karthiknadig <3840081+karthiknadig@users.noreply.github.com>
karthiknadig
added
the
bug
|
Current design could be misleading, please check my comment on the original issue: #809 (comment) |
|
Thanks @Carbaz for pointing that out. That is a great suggestion. |
This PR implements automatic detection of uv availability and updates the venv manager display name to show "venv [uv]" . Also fixes various other localized messages to include uv and clarify this to users
Fixes #809.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.