search

pypa / pipenv

Python Development Workflow for Humans.
https://pipenv.pypa.io
MIT License
24.83k stars 1.87k forks source link

fix for #2747 (needs refinement) #3263

Closed jrkong closed 5 years ago

jrkong commented 5 years ago

This PR should fix #2747.

The issue

Issue #2747 occurs as the original implementation of run_open did not pass the virtual environment variable to the editor handler.

The fix

The virtual environment is initialized using inline_activate_virtual_environment() and passed the VIRTUAL_ENV variable to the editor.

I considered refactoring run_open to share more common logic with do_run as suggested by @uranusjr but the alternate solution I came up with involved reusing click's get_editor logic to get the $EDITOR variable and to account for if its undefined and building a command which is run using do_run.

I'm not sure which implementation is better, any suggestions?

Also, is there anything I can do to improve my tests?

uranusjr commented 5 years ago

Thanks for taking the time! I made some comments inline with the changes. Regarding the test—I feel it might be worthwhile to try mocking out the editor-launching code (maybe click.echo()), and ensure the function is called with the correct environment and parameters.