Activate Conda env for new shells after UI updates#50715
Activate Conda env for new shells after UI updates#50715el-yawd wants to merge 1 commit intozed-industries:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Python Conda toolchain activation so that new terminal shells evaluate the appropriate Conda/Mamba/Micromamba shell hooks before running activate, aligning terminal behavior with Conda environment changes made via the UI (Issue #50619).
Changes:
- Expanded micromamba shell-name mapping to cover additional
ShellKindvariants. - Reworked Conda activation-script generation to inject per-shell hook evaluation for
micromamba, and forconda/mamba, before running{manager} activate .... - Switched from a single
managervariable tomanager_nameplusmanager_pathusage for hook execution.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| @@ -1398,27 +1400,107 @@ impl ToolchainLister for PythonToolchainProvider { | |||
| .filter(|name| matches!(*name, "conda" | "mamba" | "micromamba")) | |||
| .unwrap_or("conda"), | |||
| }; | |||
| let manager_path = manager_info.executable.to_string_lossy(); | |||
There was a problem hiding this comment.
manager_name can differ from manager_info.executable (e.g., when the user explicitly selects Conda/Mamba/Micromamba, or on Windows where the executable is micromamba.exe so the file_name() filter misses it). In that case the hook is evaluated using one binary (manager_path) but activation is attempted via a different command ({manager_name} activate ...), which can make activation fail. Consider deriving both the executable path and the command name from the same resolved manager (e.g., detect via file_stem()/strip .exe, and when a specific manager is configured, resolve that binary and use it consistently for both hook and activate).
| // Activate the manager in the child shell. | ||
| // This is required for `conda activate` to work. | ||
| match manager_name { | ||
| "micromamba" => { | ||
| let shell_arg = micromamba_shell_name(shell); |
There was a problem hiding this comment.
This PR changes Conda activation semantics by injecting per-shell hook evaluation before running activate, but the existing test only asserts that the environment name is quoted. Please add/extend tests to cover the new hook lines (at least for ShellKind::Posix and one non-Posix shell) and to verify the hook/activate use the same resolved manager.
|
Another one: #50895 |
…50895) Closes #50619 In the conda activation script building procedure, Zed currently performs a file check for the conda executable on the client side. When in remote development, this check always fails because the file exists on the remote host, not the local machine. Since `pet` already handles existence checks, removing this redundant check allows the activation to proceed. It is also better to let any potential issues (like permissions) show up in the terminal rather than silently skipping the activation. This addresses the root cause for remote development, which is different from the approach in #50715 that focuses on shell hooks. **The video recording** https://github.com/user-attachments/assets/62967351-e3c5-4814-aec4-b2332940e7e3 Before you mark this PR as ready for review, make sure that you have: - [x] Added a solid test coverage and/or screenshots from doing manual testing - [x] Done a self-review taking into account security and performance aspects - [x] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) Release Notes: - Fixed conda environment not auto-activating in the terminal during remote development sessions.
…ed-industries#50895) Closes zed-industries#50619 In the conda activation script building procedure, Zed currently performs a file check for the conda executable on the client side. When in remote development, this check always fails because the file exists on the remote host, not the local machine. Since `pet` already handles existence checks, removing this redundant check allows the activation to proceed. It is also better to let any potential issues (like permissions) show up in the terminal rather than silently skipping the activation. This addresses the root cause for remote development, which is different from the approach in zed-industries#50715 that focuses on shell hooks. **The video recording** https://github.com/user-attachments/assets/62967351-e3c5-4814-aec4-b2332940e7e3 Before you mark this PR as ready for review, make sure that you have: - [x] Added a solid test coverage and/or screenshots from doing manual testing - [x] Done a self-review taking into account security and performance aspects - [x] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) Release Notes: - Fixed conda environment not auto-activating in the terminal during remote development sessions.
|
Ah right, this was about the same issue that #50895 , given we merged that and it seemed to be a more concise fix I will close this PR in favor of it. Thank you for trying to tackle this either way |
Closes #50619
Before you mark this PR as ready for review, make sure that you have:
Release Notes:
rec-2026-03-04_13-29-20.mp4
I couldn't managed to remove those printings has shown above, any help on how to do it on the background or before loading would be welcome