asdf-vm/asdf

asdf.fish doesn't escape spaces when setting path

sullyj3 opened this issue · 8 comments

Steps to reproduce

Followed the installation instructions for fish

echo 'source ~/.asdf/asdf.fish' >> ~/.config/fish/config.fish
mkdir -p ~/.config/fish/completions; and cp ~/.asdf/completions/asdf.fish ~/.config/fish/completions

Expected behavior

Spaces in existing path entries should be correctly escaped

Actual behavior

When opening a new terminal, a bunch of errors:

set: Warning: $PATH entry "/mnt/c/Program" is not valid (No such file or directory)
set: Warning: $PATH entry "/mnt/c/Program" is not valid (No such file or directory)
set: Warning: $PATH entry "/mnt/c/Program" is not valid (No such file or directory)
set: Warning: $PATH entry "/mnt/c/Program" is not valid (No such file or directory)
set: Warning: $PATH entry "/mnt/c/Program" is not valid (No such file or directory)
set: Warning: $PATH entry "/mnt/c/Program" is not valid (No such file or directory)
set: Warning: $PATH entry "/mnt/c/Program" is not valid (No such file or directory)

Environment

OS: Windows 10, WSL (Ubuntu)

asdf version:
0.7.4

The problem seems to be this line:
set PATH $x (echo $PATH | command xargs printf '%s\n' | command grep -v $x)

Although I'm not immediately sure how to fix it.

I don't use fish shell, and it is not obvious to me what the problem is here. My first guess is there are missing quotes somewhere. @danhper can you look into this?

You should put newline before source
in echo 'source ~/.asdf/asdf.fish' >> ~/.config/fish/config.fish,
if you have anything in your ~/.config/fish/config.fish

As @LikoIlya mentioned, the command we recommend for Fish users doesn't add to a new line of the file as our other Bash and ZSH code snippets do:

docs: https://asdf-vm.com/#/core-manage-asdf-vm?id=add-to-your-shell

echo -e "\n. $HOME/.asdf/asdf.sh" >> ~/.zshrc

vs

echo 'source ~/.asdf/asdf.fish' >> ~/.config/fish/config.fish
mkdir -p ~/.config/fish/completions; and cp ~/.asdf/completions/asdf.fish ~/.config/fish/completions

If a Fish user comes across this issue, can they confirm whether this is the problem?

@sullyj3 Are you still experiencing this issue with the latest version? The docs for the setup of the shells has been updated to show what should be in the config file, not how to add the configuration to the file. Hopefully that clears up confusion from the problem raised by @LikoIlya

jmaya commented

I can confirm the same problem.
In my case I am using wsl 2 for windows

set: Warning: $PATH entry "/c/Program" is not valid (No such file or directory)
set: Warning: $PATH entry "/c/Program" is not valid (No such file or directory)
set: Warning: $PATH entry "/c/Users/jmaya/AppData/Local/Programs/Microsoft" is not valid (No such file or directory)
Welcome to fish, the friendly interactive shell
[I] ⋊> /c/U/jmaya fish version                                                                                  23:15:24
version: No such file or directory
[I] ⋊> /c/U/jmaya fish --version                                                                                23:16:26
fish, version 2.7.1

I was able to add this code to the offending line and fix the issue

 set PATH $x ( env | egrep '^PATH=' | awk -F= '{print $2}' | tr : "\n" | xargs -d "\n" printf '%s\n')

In short, fish is not understanding the extra spaces on windows paths. It tries to add them as separate lines. This should correct the issue.

@jmaya are you able to perform further investigation? @Stratus3D and myself are not fish users so do not have a setup readily available for debugging and testing

jmaya commented

@jthegedus @Stratus3D
Here is the diff. This worked.

index 038178d..255df3f 100644
--- i/asdf.fish
+++ w/asdf.fish
@@ -10,7 +10,7 @@ set -l asdf_bin_dirs $ASDF_DIR/bin $ASDF_DIR/shims $asdf_data_dir/shims

 for x in $asdf_bin_dirs
   if test -d $x
-    set PATH $x (echo $PATH | command xargs printf '%s\n' | command grep -v $x)
+    set PATH $x ( env | egrep '^PATH=' | awk -F= '{print $2}' | tr : "\n" | xargs -d "\n" printf '%s\n')
   end
 end

I believe this was fixed in #551 but it is hard to say given the timeline of the PR being merged before @jmaya commented.

I will close for now as we are many new versions and fixes and no one has reported an issue here in over 2 years now.