Skip to content

Conversation

nickgnd
Copy link

@nickgnd nickgnd commented Oct 18, 2025

Hey 👋
I spontaneously decided to give it a try to #171 and see if i can fix Fish support. I hope this doesn't bother you, if so, feel free to close the PR.


Fish shell uses space-separated PATH variables internally (as a proper list), while other shells like bash and zsh use colon-separated strings. When Expert tried to detect the Elixir executable by running echo $PATH in Fish, it received space-separated output like: /opt/homebrew/bin /opt/homebrew/sbin /Users/username/.local/bin ...

This caused :os.find_executable/2 to fail since it expects colon-separated paths on Unix systems, preventing Fish users from using Expert properly.

Proposed Solution

This PR adds Fish-specific handling to convert the PATH to the expected colon-separated format:

  • Detects Fish shell using String.contains?(shell, "fish")
  • For Fish: uses string join ':' $PATH to convert Fish's list format to colon-separated
    • string join ... is built-in in Fish and available since version 2.3.0 (2016)
  • For other shells: continues using echo $PATH as before

💡 This approach correctly handles paths with spaces in them (e.g., /Applications/Visual Studio Code.app/bin) because it works with Fish's native list structure rather than trying to parse space-separated strings.

Testing

Tested with Fish shell + Mise on macOS, where Elixir executable is now correctly found 🚀

Let me know if the solution is good enough for you, or if is there anything missing.

Cheers ✌️

Copy link
Collaborator

@doorgan doorgan left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! I left a few nitpicky comments but I tested this and it does solve the issue. I can address them and merge this when I get back later today

{path, 0} = System.cmd(shell, ["-i", "-l", "-c", "cd #{root_path} && echo $PATH"])
# Ideally, it should contain the path to shell (e.g. `/usr/bin/fish`),
# but it might contain only the name of the shell (e.g. `fish`).
is_fish? = String.contains?(shell, "fish")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can use Path.basename/1 here instead

# Fish uses space-separated PATH, so we use the built-in `string join` command
# to join the entries with colons and have a standard colon-separated PATH output
# as in bash, which is expected by `:os.find_executable/2`.
path_command = if is_fish?, do: "string join ':' $PATH", else: "echo $PATH"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: we prefer something like fish_shell?, is_x? mixes both naming conventions for guards and booleans

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants