virtualfish icon indicating copy to clipboard operation
virtualfish copied to clipboard

Support .project files in virtualenvs if they are available

Open dwt opened this issue 7 years ago • 26 comments

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!

dwt avatar Dec 07 '17 19:12 dwt

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.

justinmayer avatar Dec 07 '17 20:12 justinmayer

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 avatar Dec 08 '17 09:12 dwt

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

dwt avatar Dec 12 '17 08:12 dwt

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.

justinmayer avatar Dec 13 '17 18:12 justinmayer

@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.

dwt avatar Dec 18 '17 10:12 dwt

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.

daisylb avatar Dec 27 '17 00:12 daisylb

Adam Brenecki wrote:

I'm also open to hearing the argument for vf activate automatically cding like vf workon does now

Personally, I like that I have a way of activating a virtual environment without any directory switching. I often find myself in the right directory but just need to activate a particular virtual environment, but one that is usually associated with a different project. So the current “vf activate” behavior is quite handy for those situations. Otherwise one has to activate and then manually switch back to where they were before the auto-cd happened.

Just my two cents. 😁

justinmayer avatar Dec 27 '17 04:12 justinmayer

@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 avatar Dec 27 '17 14:12 dwt

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 avatar Dec 28 '17 22:12 dwt

@justinmayer + @adambrenecki Any input?

dwt avatar Jan 03 '18 19:01 dwt

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.

justinmayer avatar Jan 03 '18 21:01 justinmayer

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 avatar Apr 23 '18 16:04 schettino72

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

schettino72 avatar Apr 23 '18 16:04 schettino72

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

asfaltboy avatar Mar 24 '19 14:03 asfaltboy

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.

dwt avatar Mar 25 '19 09:03 dwt

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: https://github.com/justinmayer/virtualfish/commit/4f87c8e1f91dcb2ca7b10fe8c156cd9b92bef10e

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?

justinmayer avatar Mar 25 '19 14:03 justinmayer

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.

dwt avatar Mar 26 '19 07:03 dwt

@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.

justinmayer avatar Mar 26 '19 10:03 justinmayer

Seems to work fine for me

dwt avatar Mar 26 '19 12:03 dwt

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

justinmayer avatar Mar 29 '19 09:03 justinmayer

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?

justinmayer avatar Apr 02 '20 15:04 justinmayer

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.

dwt avatar Apr 02 '20 19:04 dwt

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.

justinmayer avatar Apr 02 '20 19:04 justinmayer

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!

cecep2 avatar May 19 '20 14:05 cecep2

@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.

justinmayer avatar May 19 '20 17:05 justinmayer

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.

dwt avatar Sep 05 '23 09:09 dwt