Skip to content
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

Change behaviour significantly: make work for all cases #2

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dgrant
Copy link

@dgrant dgrant commented Dec 3, 2019

-Start at current directory
-Look inside all sub-directories to see if there is a bin/activate.fish script
-If not found, look in the parent directory, and continue up the file path until one is found, or give up

Covers cases like:
-Virtual environment is in a .venv or venv folder (any folder name is supportd) in the current working directory
-Virtual environment is in a parent folder of the current working directory
-Virtual environment is in the current working directory

-Start at current directory
-Look inside all sub-directories to see if there is a bin/activate.fish script
-If not found, look in the parent directory, and continue up the file path until one is found, or give up

Covers cases like:
-Virtual environment is in a .venv or venv folder (any folder name is supportd) in the current working directory
-Virtual environment is in a parent folder of the current working directory
-Virtual environment is in the current working directory
@dgrant
Copy link
Author

dgrant commented Dec 3, 2019

Fixes #3

@luispabon
Copy link

Much better solution 👍

@dgrant
Copy link
Author

dgrant commented Dec 3, 2019

@luispabon if you are able to test that would be great

I'm not sure how you manage plugins, but I use fisher and so all I had to do was remove the old one, and then do fisher add ~/git/fish-autovenv where ~/git/fish-autovenv was pointing to this branch.

@dgrant
Copy link
Author

dgrant commented Dec 3, 2019

@timothybrown if you can test as well that would be much appreciated.

@dgrant
Copy link
Author

dgrant commented Dec 3, 2019

@timothybrown actually I think this might not work for your case anymore, if you are in a directory above your virtualenv(s)/project, then it will find the first project/env subdir and switch to it. I don't think this would be disastrous though.

And I think it's important to note that putting your project files inside the virtualenv is not a recommended practice, see here: https://stackoverflow.com/questions/1783146/where-in-a-virtualenv-does-the-custom-code-go

"Because virtualenvs are not relocatable, in my opinion it is bad practice to place your project files inside a virtualenv directory"

"But strongly disagree about EVER putting your code inside the virtual environment. If you want it "near" the project on the filesystem then put a venv/ directory on the same level as the project's BASE_DIR"

Also, it can be common to have a few different python versions to test with. It makes sense to have a venv-2.7 for example and a venv-3.7 for example. And switch between each for testing. The whole point of virtualenv is to de-couple python version from your app. The way to do this is to have it as a completely separate directory somewhere, or as a sub-directory. Also the point of virtualenv is to make it easy to completely dismantle your environment and bring it up again from scratch. It's not possible to do this if your virtualenv and project are in the same directory.

@dgrant
Copy link
Author

dgrant commented Dec 17, 2019

@timothybrown Hey timothy can we merge this?

We have me, @luispabon, and @matthewess who are interested in this

@kjkta
Copy link

kjkta commented Apr 20, 2020

Works great here!

Installed with omf omf install https://github.com/dgrant/fish-autovenv.git

@dgrant
Copy link
Author

dgrant commented Apr 20, 2020

Glad some people like this change! Perhaps I should rename mine to fish-autovenv2 as it's truly a fork at this point as I've merged to my master and this original one doesn't seem to be getting any love.

@m-radzikowski
Copy link

Would be great to have this merged, as I was really disappointed after installing autovenv and started digging why it does not work as expected only to find this PR.

@timothybrown ?

@luispabon
Copy link

This is working correctly for me regardless of the location of the venv 👍

@dgrant
Copy link
Author

dgrant commented Apr 18, 2021

Hey everyone, I've renamed the repo to https://github.com/dgrant/fish-autovenv2

omf install https://github.com/dgrant/fish-autovenv.git should still work thanks to redirects

omf install https://github.com/dgrant/fish-autovenv2.git should work too.

I'm submitted it to the omf main packages repository here: oh-my-fish/packages-main#140

So I've you'd like to see it added put a comment there or an emoji or something.

@ml31415
Copy link

ml31415 commented Nov 14, 2023

I'd suggest to use only wellknown .vevn and .virtualenv folder for autodetection and others folder name as configuration option. Right now chances are, it activates venvs even if you don't like to. Also, right now no symlink that might lead to a venv isn't searched.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants