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

Add --here option for pew workon to prevent current directory to be changed #180

Merged
merged 1 commit into from
Mar 27, 2018
Merged

Add --here option for pew workon to prevent current directory to be changed #180

merged 1 commit into from
Mar 27, 2018

Conversation

AlexandreDecan
Copy link
Contributor

@AlexandreDecan AlexandreDecan commented Mar 6, 2018

This PR implements #179.

It adds an optional argument for pew workon (--here) that prevents the current directory to be changed.

@AlexandreDecan AlexandreDecan changed the title Add -h / --here option for workon to stay in current directory Add --here option for pew workon to prevent current directory to be changed Mar 6, 2018
@berdario
Copy link
Collaborator

berdario commented Mar 7, 2018

Thank you

What is the use case? If you created the venv yourself, probably you can simply avoid creating it as a project if you don't want the cd functionality

I guess this could be useful if you're working on a shared machine, but I fail to see exactly the need for this.

Also, I haven't checked, but I think that pew in environ $SHELL would already not automatically cd

@AlexandreDecan
Copy link
Contributor Author

Basically, this PR allows the workflow "cd into the right directory; then enable venv with no cd" where one had to follow "enable venv with cd; cd into the right directory". As, most of the time, I first open a terminal in the right directory and then I enable the environment, I thought it could be interesting to add this option to pew workon.

Another way of seeing this PR is to consider it as a way to allow a "default path" for a venv, with an easy way to reuse a single venv for different purpose.

@berdario
Copy link
Collaborator

Sorry, I haven't merged this yet.

I still feel a bit wavering about this, I hoped that in the last couple of weeks someone would chime in and give his perspective on this, if they'd think this would be useful or not.

@AlexandreDecan
Copy link
Contributor Author

No problem, I understand;)

@meshy
Copy link
Collaborator

meshy commented Mar 27, 2018

FWIW, I'd like this.

At the moment, when pipenv creates environments, it uses pew under the hood, but it doesn't pass -a $(pwd). I'm hoping to create a PR to remedy this soon, but I'm concerned that I will (justifiably) get push-back from their end, because it would mean that pew workon myproject would have different behaviour as a result.

Having this flag available on pew would mean that I would be able to offer an alternative workflow to pipenv when I suggest the change.

@berdario
Copy link
Collaborator

I see, this makes sense.

Thinking more about it, while the project/-a feature can be useful, I'm now wondering if this was actually a good idea: have some non-obvious (since it wouldn't be in a centralized place, but rather a file in every virtualenv) configuration that changes the default behavior of the tool.

Rather than having a flag to restore the default behavior, maybe it would've been better to have a --cd flag that would allow people to opt-in, whenever they'd want to cd into the preselected project directory.

Alas, the choice has been made (mostly because pew started just as a virtualenvwrapper reimplementation) and breaking compatibility for something like this is an even worse idea

@meshy
Copy link
Collaborator

meshy commented Mar 27, 2018

I think you're right. Ideally, the --cd option would be saner.

It's pretty late for me, so this might just be me sleep-typing, but I have an idea that might work:

We could transition to this better behaviour by always creating an extra file in the virtual environments (eg: .pew, as well as .project).

When .project is found, we would know that -a was passed, and always change directories.

Otherwise, when --cd is passed, we could switch to the path in .pew.

Oh, and .pew probably isn't a good name for the file.

@meshy
Copy link
Collaborator

meshy commented Mar 27, 2018

Actually, thinking about it more, in light of pypa/pipenv#1824, perhaps it would be of use to populate a single file in $WORKON_HOME with all the venvs and any associated config.

@meshy
Copy link
Collaborator

meshy commented Mar 27, 2018

There is another possible advantage to having a new file format for this information (a single-file or one-per-project doesn't matter) -- each project could individually configure if it should switch by default.

I have no idea if that's something people would use though.

@berdario berdario merged commit a61d0ee into pew-org:master Mar 27, 2018
@berdario
Copy link
Collaborator

Yup, anyhow let's stick to the minimal implementation for now.

I merged it, and just added a couple of TODO notes and tests

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.

3 participants