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 environment plugin #119

Merged
merged 7 commits into from Jun 11, 2017
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 37 additions & 10 deletions virtualfish/environment.fish
@@ -1,6 +1,30 @@
# Set up the configurable filename
if not set -q VIRTUALFISH_ENVIRONMENT_FILE
set -g VIRTUALFISH_ENVIRONMENT_FILE .env
end

function __vfsupport_set_env_file_path --description "Set VIRTUALFISH_ENVIRONMENT_FILE_PATH to appropriate value"
set -l vf_base (basename $VIRTUAL_ENV)
set -l vf_path $VIRTUAL_ENV/$VIRTUALFISH_ENVIRONMENT_FILE
# Check if projects plugin is used
if set -q PROJECT_HOME
set -l project_path $PROJECT_HOME/$vf_base/$VIRTUALFISH_ENVIRONMENT_FILE
if not test -r $project_path
and test -r $vf_path
set -g VIRTUALFISH_ENVIRONMENT_FILE_PATH $vf_path
else
set -g VIRTUALFISH_ENVIRONMENT_FILE_PATH $project_path
end
else
set -g VIRTUALFISH_ENVIRONMENT_FILE_PATH $vf_path
end
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably use echo instead of setting a global variable. I also realized the basename call doesn't need to be made if $PROJECT_HOME is not defined, so that should probably be moved further below. I also (very slightly) simplified the environment file path determination logic:

function __vfsupport_env_file_path --description "Return full path to environment file"
    set -l vf_env_file_path $VIRTUAL_ENV/$VIRTUALFISH_ENVIRONMENT_FILE
    # Check if Projects plugin is used
    if set -q PROJECT_HOME
        set -l vf_name (basename $VIRTUAL_ENV)
        set -l proj_env $PROJECT_HOME/$vf_name/$VIRTUALFISH_ENVIRONMENT_FILE
        if test -r $proj_env
            echo $proj_env
        else
            echo $vf_env_file_path
        end
    else
        echo $vf_env_file_path
    end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like your echo suggestion a lot - that does exactly what I was searching for. The reason I used more complicated logic when the Projects plugin is present was for creating a new env file. Here's how my thinking goes:

  1. If we aren't using Projects, place the .env file in the virtualenv directory
  2. If we are using Projects and there is a .env in the project directory, use that file
  3. If we are using Projects and there is not a .env file in the project directory and there is and .env file in the virtualenv directory use that.
  4. (This is why it got a little more complicated) If we are using Projects and there isn't a .env file in either the project or virtualenv directories, create a new file in the project directory.
    Case 4 is why I have the logic like this:
if set -q PROJECT_HOME
    if not test -r $proj_env
        and test -r $vf_env
        # Use the virtualenv only when that's been established with a file there
    else
        # Prefer project directory
    end
else
    # Not using Projects
end

I think adding some comments and perhaps reordering the logic might make it a bit more clear.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I hadn't considered that the logic was taking environment file creation into account. Now your logic makes more sense. I agree some comments might help clarify.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't until I was working on the environment command that I realized that was an issue, just because it opens the file in the $EDITOR regardless of whether it exists. Kind of an edge case, but it caught me. 😄


function __vfext_environment_activate --on-event virtualenv_did_activate
if test -f $VIRTUAL_ENV/virtualfish-environment
for line in (cat $VIRTUAL_ENV/virtualfish-environment)
__vfsupport_set_env_file_path

if test -r $VIRTUALFISH_ENVIRONMENT_FILE_PATH
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section would become:

set -l env_file_path (__vfsupport_env_file_path)

if test -r $env_file_path

while read -l line
# Skip empty lines and comments
if not string length -q $line
or test (string sub -s 1 -l 1 $line) = "#"
Expand All @@ -12,18 +36,20 @@ function __vfext_environment_activate --on-event virtualenv_did_activate

# Preserve existing env var with shared name
if set -q $key
set -xg __VF_ENVIRONMENT_OLD_VALUE_$key $$key
set -gx __VF_ENVIRONMENT_OLD_VALUE_$key $$key
end

# Eval to allow for expanding variables, e.g. PATH=$PATH foo
set -gx $key (eval echo $value)
end
end < $VIRTUALFISH_ENVIRONMENT_FILE_PATH
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

end < $env_file_path

end
end

function __vfext_environment_deactivate --on-event virtualenv_will_deactivate
if test -f $VIRTUAL_ENV/virtualfish-environment
for line in (cat $VIRTUAL_ENV/virtualfish-environment)
__vfsupport_set_env_file_path

if test -r $VIRTUALFISH_ENVIRONMENT_FILE_PATH
while read -l line
# Skip empty lines and comments
if not string length -q $line
or test (string sub -s 1 -l 1 $line) = "#"
Expand All @@ -35,20 +61,21 @@ function __vfext_environment_deactivate --on-event virtualenv_will_deactivate

# Check if old value was preserved
if set -q $old_key
set -xg $key $$old_key
set -gx $key $$old_key
set -e $old_key
else
set -e $key
end
end
end < $VIRTUALFISH_ENVIRONMENT_FILE_PATH
end
end

function __vf_environment --description "Edit the environment variables for the active virtual environment"
if set -q 'VIRTUAL_ENV'
eval $EDITOR $VIRTUAL_ENV/virtualfish-environment
if set -q VIRTUAL_ENV
__vfsupport_set_env_file_path
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set -l env_file_path (__vfsupport_env_file_path)

# Deactivate before applying new env vars to avoid stomping stashed values
__vfext_environment_deactivate
eval $EDITOR $VIRTUALFISH_ENVIRONMENT_FILE_PATH
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eval $EDITOR $env_file_path

__vfext_environment_activate
else
echo "Must have a virtual env active to run command"
Expand Down