justinmayer / virtualfish

Fish shell tool for managing Python virtual environments
MIT License
1.06k stars 100 forks source link

Auto-Activation: Docs about setting $PATH in config.fish #180

Closed raphant closed 4 years ago

raphant commented 4 years ago

Issue

When this plugin is enabled, ensure any modifications to your $PATH in your config.fish happen before VirtualFish is loaded. From Plugins This seems unclear. How can one ensure that the $PATH modifications happen before VirtualFish?

This is my current configuration. Does this satisfy that requirement?

set -xU RANGER_LOAD_DEFAULT_RC FALSE
set -gx PATH $HOME/.local/bin $PATH
set -gx EDITOR vim
abbr setclip "xclip -selection c"
abbr getclip "xclip -selection c -o"
abbr du "du -h"
justinmayer commented 4 years ago

That note was added to the docs in response to #62, which was written when VirtualFish used a different loading mechanism and thus no longer seems to be appropriate. I just changed that note in the docs via 110c0a4. See also: #176

I don't use the Auto-Activation plugin and thus do not know how $PATH settings can affect its behavior. Unlike other parts of VirtualFish, the Auto-Activation plugin can cause VirtualFish functions to be invoked before config.fish has loaded, so that plugin is more sensitive to where configuration variables are set. If you were working in a directory that contained a .venv file and then opened a split terminal window that inherited that directory, it is likely that the virtual environment will be activated before config.fish has loaded, which could result in $HOME/.local/bin appearing before (and thus taking precedence over) ~/.virtualenvs/foo/bin, which may or may not pose a problem depending on whether there are executables with the same name in both places.

In summary, I imagine your configuration is fine and don't think you need to change anything about in it unless you run into a problem. Does that help clarify things?

justinmayer commented 4 years ago

As noted above, I have since changed the docs, so this issue has ostensibly been resolved.