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

Support .project files in virtualenvs if they are available #134

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

dwt
Copy link
Contributor

@dwt dwt commented Dec 7, 2017

Hi there,

since this is a feature of virtualenvwrapper that is quite important to me, I added support for it to virtual fish.

Would you please merge it?

Best of thanks!

@justinmayer
Copy link
Owner

While for me the jury is still out regarding the viability of this endeavor, I understand your desire to support your personal work-flow. Any such attempt, however, should probably occur within the context of the relevant plugin and not in virtual.fish.

@dwt
Copy link
Contributor Author

dwt commented Dec 8, 2017

Do I get this right? You would take the pull request if it is implemented inside the projects plugin, instead of in the global activate function?

@dwt
Copy link
Contributor Author

dwt commented Dec 12, 2017

@justinmayer Please clarify - so I know it is worth putting more work into this.

@justinmayer
Copy link
Owner

I am open-minded about it, provided the implementation does not cause problems for existing behaviors. That said, the final decision lies with @adambrenecki and is not mine to make.

@dwt
Copy link
Contributor Author

dwt commented Dec 18, 2017

@adambrenecki how do you feel about this? I'd like to get the fish version to be feature compatible with virtualenvwrapper to make it as easy as possible to switch.

@daisylb
Copy link
Collaborator

daisylb commented Dec 27, 2017

Yep, I'd prefer this to be part of the projects plugin, I think. The way I see it, the .project file should act as an override over the default behaviour of $PROJECT_HOME/(basename $VIRTUAL_ENV) being the project path. So, vf cdproject should work with .project files, and the automatically-cd behaviour should be part of vf workon rather than vf activate.

I'm also open to hearing the argument for vf activate automatically cding like vf workon does now—I never used VEW's projects features, and I don't use the vf projects plugin, so I don't know if that would be useful or annoying—but in any case it should behave the same regardless of the presence of .project, and should be implemented using hooks in the projects plugin.

@justinmayer
Copy link
Owner

justinmayer commented Dec 27, 2017 via email

@dwt
Copy link
Contributor Author

dwt commented Dec 27, 2017

@justinmayer: I actually use cd - quite often for that. But I agree, it seems logical that vf activate only activates a virtualenv, while workon also does a cd to the env, just like virtualenvwrapper does.

@dwt
Copy link
Contributor Author

dwt commented Dec 28, 2017

I have updated this pull request with a tentative implementation for a more complete .project support.

vf new virtualenvname -a path/to/project is supported, as is vf project virtualenvname -a path/to/project

I'm not entirely happy with the implementation yet, but that is also in large parts because I don't know fish very well and am surely doing stuff the wrong way. So any advice is much appreciated.

Some closing thoughts: It could be interesting to go for a more complete embrace of the way virtualenvwrapper handles projects, to try to be as compatible as possible and always uses the .project file to locate projects. This would of course break existing workflows which I dislike - but at the cost of more complex code and a behavior that differs from virtualenvwrapper making it harder to switch. So maybe worth a thought?

@dwt
Copy link
Contributor Author

dwt commented Jan 3, 2018

@justinmayer + @adambrenecki Any input?

@justinmayer
Copy link
Owner

Hi Martin. My schedule is completely booked at the moment, so I may not be able to adequately review this pull request for a while. I'll do my best to have a look when the dust settles.

That said, I will take a moment to comment regarding compatibility with virtualenvwrapper: I do not think this should be a priority goal for this project. The existing compatibility aliases serve as "training wheels" and ease the transition for folks coming from virtualenvwrapper (as I myself once did). But just as the Fish shell community rightly declined to add Bash compatibility with the rationale that Fish incompatibility with Bash is a feature, not a bug, I think there are a lot of things in virtualenvwrapper that should not be in Virtualfish. Rather than compatibility with another project, I believe that potential changes to Virtualfish should be primarily viewed through the lens of whether they improve this project.

@schettino72
Copy link

Just my 2 cents regarding this issue. I am switching from virtualenwrapper to vf and I am not interested in using the workon compatible plugin.

But the ability to optionally define a project path and automatically switch to that folder is pretty a nice feature. I use this in some of my virtualenvs but not all...

This feature wont change behaviour if your virtualenv was create by vf because the .project file wont exist.

@schettino72
Copy link

For those interested waiting for a resolution on this issue. It can be achieved with a function that watches $VIRTUAL_ENV:

function _venv_cd_project_on_activate --on-variable VIRTUAL_ENV
    if test -e $VIRTUAL_ENV/.project
        cd (cat $VIRTUAL_ENV/.project)
    end
end

@asfaltboy
Copy link

I have a similar workflow, or rather I like to arrange projects into subdirectories according to category / tag (e.g work, experiments, github, etc...) and not having the ability to change the home dir makes things difficult.

I rebased this PR over latest master and tested it, it works well for me. Cannot push to PR as I'm not a maintainer, but my rebased for is here if needed: asfaltboy/virtualfish#dwt-master

@dwt
Copy link
Contributor Author

dwt commented Mar 25, 2019

I have since not switched to virtual fish, so I'm not using this yet (also because of the problems) but my thinking is now, that it may be best to just store to .project all the time and use that as the default storage instead of an additional one. That removes custom arguments and makes the implementation smoother. (just needs an initializer logic for virtualenvs that are missing that file.

@justinmayer
Copy link
Owner

Hi folks. I appreciate the work Martin has put into this, and clearly there are people who want to use .project files. Part of the delay in reviewing this endeavor is the scope — not that it's inherently expansive, but it includes just enough to add cognitive friction and make it harder for time-starved folks like me to evaluate and merge the contribution. In general, my preferred approach would be iterative, starting out with the smallest, minimally-viable implementation.

So I took a stab at a minimal implementation, also adding some error-checking in case folks put a non-existent directory into the .project file: 4f87c8e

This would allow users to begin using .project files right away, without having to first sort out some of the thornier issues regarding how they are generated. In the interim, folks could for example invoke echo $PWD > $VIRTUAL_ENV/.project to associate the current directory with the currently-active virtual environment.

What do you think?

@dwt
Copy link
Contributor Author

dwt commented Mar 26, 2019

I really like this. It makes the .project files that are already existing work right away and is a great first step.

About creating .project files:

  • The least intrusive option could be to create a new command that associates the current directory via a .project file. That works and essentially does echo $PWD > $VIRTUALENV/.project. Good: no change to behaviour of existing commands. Bad: Adds a new command and needs to be documented on the existing commands anyway to make it easily discoverable. Maybe this could be a next step that is relatively easy to agree on though.
  • A more intrusive option would be to add an option to the existing commands. This is easier to discover but adding options seems not super easy in fish looking at my last attempt.

@justinmayer
Copy link
Owner

@dwt: Glad to hear it. I added this via PR #149, including your fix for project completions when PROJECT_HOME is not set or available. Please have a look and let me know if anything looks awry.

My initial impression is that the less-intrusive option is probably the best way forward for now. In fact, I wonder whether it is just as easy for users (and certainly easier for us) to put the echo $PWD > $VIRTUALENV/.project command in the documentation. So for now, I've added a commit (fce7373) that does exactly that.

@dwt
Copy link
Contributor Author

dwt commented Mar 26, 2019

Seems to work fine for me

@justinmayer
Copy link
Owner

Great. Initial support for .project files is now in master. 🎊

@justinmayer
Copy link
Owner

This initial support for .project files was recently released as part of VirtualFish 2.0. 🎉

If @dwt or anyone else would like to propose follow-up enhancements, would you please suggest here what those might be, so we can determine the next course of action?

@dwt
Copy link
Contributor Author

dwt commented Apr 2, 2020

Great to hear. I'm currently quite deeply involved in building anonymous and privacy preserving bluetooth tracing applications to fight covid19 - but I'll get back here as soon as I get a little bit downtime.

@justinmayer
Copy link
Owner

That is certainly a more important endeavor in these troubled times. Glad to hear you are working on something that will hopefully make a difference. Looking forward to your return after some calm has been restored.

@cecep2
Copy link
Contributor

cecep2 commented May 19, 2020

If @dwt or anyone else would like to propose follow-up enhancements, would you please suggest here what those might be, so we can determine the next course of action?

I usually would open a new issue, but because you invite suggestions here: I would love auto-deactivation of the virtualenv when I cd out of the project directory - just like the auto-activation plugin does when I cd out of a directory containing a .venv file.

In any case, thank so much for this great work!

@justinmayer
Copy link
Owner

@cecep2: Thank you for the kind words. I wrote the Projects plugin, but I don't use the auto-activation plugin and thus am ill-equipped to comment on how the two plugins might be combined to provide the functionality you mentioned. That said, I can't imagine it would be hugely difficult to enhance the Projects plugin to determine whether the auto-activation plugin has been loaded, and if so, de-activate the associated virtual environment upon leaving the project directory.

Is this something you would be willing to help out with? If so, I would be happy to provide guidance to assist you in implementing this feature.

@dwt
Copy link
Contributor Author

dwt commented Sep 5, 2023

I have to say, I'm using vf with projects support for quite some time now, and have gotten into the habit of creating the .project file with this shell shortcut, after creating and activating a new venv:

pwd >  $VIRTUAL_ENV/.project

That has quite stalled my energy to enhance this pull request. :/ But maybe this shortcut can be useful to other people in the meantime.

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.

None yet

6 participants