deactivate creates a new shell

[memeplex]

Type: Bug

Behaviour

Expected vs. Actual

After the changes described in #22448, deactivate is creating a new shell. This shouldn't be the case given that, at least, it implies:

1. Recent command history is lost.
2. Unexported shell environment is lost.
3. Extra C-d are required in order to close the terminal.

Steps to reproduce:

1. Create a new terminal.
2. Check that the venv is set.
3. echo $$ -> PID X
4. deactivate
5. echo $$ -> PID Y
6. C-d to exit
7. echo $$ -> PID X again
8. C-d to exit (now the terminal is actually closed)

Diagnostic data

• Python version (& distribution if applicable, e.g. Anaconda): 3.11.5
• Type of virtual environment used (e.g. conda, venv, virtualenv, etc.): Venv
• Value of the python.languageServer setting: Default

Output for Python in the Output panel (View→Output, change the drop-down the upper-right of the Output panel to Python)

XXX

User Settings

Multiroot scenario, following user settings may not apply:

venvFolders: "<placeholder>"

languageServer: "Pylance"

Extension version: 2023.22.1
VS Code version: Code 1.85.2 (8b3775030ed1a69b13e4f4c628c612102e30a681, 2024-01-18T06:40:11.430Z)
OS version: Darwin arm64 23.3.0
Modes:

System Info

Item	Value
CPUs	Apple M1 (8 x 24)
GPU Status	2d_canvas: enabled canvas_oop_rasterization: enabled_on direct_rendering_display_compositor: disabled_off_ok gpu_compositing: enabled multiple_raster_threads: enabled_on opengl: enabled_on rasterization: enabled raw_draw: disabled_off_ok video_decode: enabled video_encode: enabled vulkan: disabled_off webgl: enabled webgl2: enabled webgpu: enabled
Load (avg)	1, 2, 2
Memory (System)	16.00GB (0.07GB free)
Process Argv	--crash-reporter-id 069181bb-01be-4b93-ad6a-50892a6240d6
Screen Reader	no
VM	0%

A/B Experiments

vsliv368cf:30146710
vspor879:30202332
vspor708:30202333
vspor363:30204092
vswsl492cf:30256860
vscorecescf:30445987
vscod805:30301674
binariesv615:30325510
vsaa593cf:30376535
py29gd2263:30899288
vscaat:30438848
c4g48928:30535728
azure-dev_surveyone:30548225
a9j8j154:30646983
962ge761:30951796
pythongtdpath:30769146
welcomedialogc:30910334
pythonidxpt:30866567
pythonnoceb:30805159
asynctok:30898717
pythontestfixt:30902429
pythonregdiag2:30936856
pyreplss1:30897532
pythonmypyd1:30879173
pythoncet0:30885854
pythontbext0:30879054
dsvsc016:30899300
dsvsc017:30899301
dsvsc018:30899302
3ef8e399:30949928

[memeplex]

I understand the limitation here since you prefer not to instruct the user to modify ~/.bashrc neither you can add a function to the shell environment, but I'd prefer to source a handy script like in source deactivate over silently losing my environment, command history, etc. Or a simple modification to my shell config, like adding an alias to source such script, or perhaps a shell function that runs:

. $VIRTUAL_ENV/bin/activate
deactivate

[karrtikr]

Hi @memeplex , you're spot on with your observations. Creating a new subshell is required to support "deactivate" command in-box without needing for users modify their init file.

In your case if you would prefer us not launching a new shell, you can follow the instructions here to set up the deactivate command for your shell: https://github.com/microsoft/vscode-python/wiki/Fixing-%22deactivate%22-command-for-Virtual-Environments.

This would set up a "deactivate" shell hook, which would be run before our own deactivate script is run, thereby solving your issue.

[memeplex]

Wouldn't it be desirable to at least show some warning like "You're now in a subshell, press Ctrl-D to return to your previous environment", maybe linking to the wiki page you've mentioned? It doesn't feel right to throw the user into a new environment without notice.

[karrtikr]

Based on majority use cases with command and the fact that number of users who actually use it are quite few, for now we're focusing on making the experience seamless. But that's good feedback, and we can decide to do that iteratively based on user reports. The environment variables though as I believe should still get carried over.

[memeplex]

The environment variables though as I believe should still get carried over.

Unfortunately, that's not the case except for exported variables. That's why I'm suggesting to be more explicit. Changing shells like this may be dangerous, removing variables and functions from the environment or, worst, changing/reverting their values. Personally, I believe it's not worth the little convenience of having deactivate at hand, moreover when you are changing the semantics of an already established command. But it's fine if we don't agree on the merits of the approach, provided that no silent destructive changes happen.