Skip to content
This repository has been archived by the owner on Dec 5, 2023. It is now read-only.

Improve 'deactivate' method to support inherited venvs #34

Closed

Conversation

brianclements
Copy link
Contributor

Completely ravamped the way the plugin remembers old paths and
reinstates them. Instead of a "stash and replace" method for the entire
path (which accidently remembers inherited venv paths), now it will:

  1. Detect inherited venvs using $VIRTUAL_ENV, remember them and set them
    as the "current venv"
  2. Deactivate any previous (inherited or former "current" venvs) before
    activating any new ones
  3. and do so by way of search and remove through the entire path for the
    paths we want to remove.

This method allows for a much more robust workflow where one can be free
to start/stop/switch the same venvs from the shell or within vim.

Also, I've decided to depreciate the redundant (and not so robust) way
of checking and prepending the new $PATH in virtualenv#activate(). As
already stated in the comments, 'activate_this.py' already does this.

Completely ravamped the way the plugin remembers old paths and
reinstates them. Instead of a "stash and replace" method for the entire
path (which accidently remembers inherited venv paths), now it will:

1. Detect inherited venvs using $VIRTUAL_ENV, remember them and set them
as the "current venv"
2. Deactivate any previous (inherited or former "current" venvs) before
activating any new ones
3. and do so by way of search and remove through the entire path for the
paths we want to remove.

This method allows for a much more robust workflow where one can be free
to start/stop/switch the same venvs from the shell or within vim.

Also, I've decided to depreciate the redundant (and not so robust) way
of checking and prepending the new $PATH in virtualenv#activate(). As
already stated in the comments, 'activate_this.py' already does this.

" DEPRECIATED: creates duplicate entries in $PATH
" the 'activate_this' script does this already.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I'm not fond of this piece of code anyway.

@jmcantrell
Copy link
Owner

I made a recent commit that's busted this. Maybe you can take a look reconciling those changes with this? I think I get where you're going with this, but I'd like to play with the PR locally.

@jmcantrell jmcantrell closed this Aug 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants