oh-my-fish / theme-bobthefish

A Powerline-style, Git-aware fish theme optimized for awesome.
MIT License
1.44k stars 222 forks source link

Commit e711caa breaks fish prompt on WSL in git repositories #360

Open Kintar opened 3 months ago

Kintar commented 3 months ago

Commit e711caa breaks the fish prompt when in a git repository. Rather than displaying the appropriate prompt, a message "wslpath: " appears.

bobthecow commented 3 months ago

@millimoose any ideas?

millimoose commented 3 months ago

@millimoose any ideas?

@bobthecow my first guess is there is no Git installed under Windows and for some reason it couldn't find one under WSL either

I'll take a look at this soon


@Kintar do you have Git installed under WSL?

Kintar commented 3 months ago

@millimoose : I have Git installed both under Windows and WSL. From a powershell terminal:

kinta@Behemoth>
➜  git status
fatal: not a git repository (or any of the parent directories): .git
kinta@Behemoth>
➜  wsl
Welcome to fish, the friendly interactive shell
Type help for instructions on how to use fish
kintar@Behemoth /m/c/u/kinta> git status
fatal: not a git repository (or any parent up to mount point /mnt)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
Kintar commented 3 months ago

@millimoose For what it's worth, I've verified that simply commenting out the line introduced in fish_prompt.fish for this commit -- command -q wslpath; and set -l git_toplevel (command wslpath $git_toplevel) -- resolves my issue.

millimoose commented 3 months ago

@Kintar oh I trust that it does, it's just baffling that the return above it wouldn't have prevented it from being hit given that you do have WSL Git

Trying Windows Git is meant to be a fallback for when WSL Git can't be found

Anyway I have something to go off of, worst case I suppose this can be hidden behind some sort of feature flag.

BlakeBLuther commented 3 months ago

I think what's happening here isn't git specific, I think it's just an improper invocation of wslpath. The issue is, I think, that wslpath is printing a filepath to stderr when the input filepath is a linux-style path. It's expecting a windows path:

Usage:
    -a    force result to absolute path format
    -u    translate from a Windows path to a WSL path (default)
    -w    translate from a WSL path to a Windows path
    -m    translate from a WSL path to a Windows path, with '/' instead of '\'

Tested on a fresh install of Ubuntu-WSL with Fish, omf, and bobthefish (slightly edited for readibility):

 ~/p/foo $ pwd
/home/platestacker/projects/foo
wslpath: /home/platestacker/projects/foo
 ~/p/foo $ set -l git_toplevel (command git rev-parse --show-toplevel 2>/dev/null)
wslpath: /home/platestacker/projects/foo
 ~/p/foo $ echo $git_toplevel
/home/platestacker/projects/foo
wslpath: /home/platestacker/projects/foo

Feeding the path directly to wslpath gives an error exit code, and redirecting stderr to /dev/null doesn't show the repeat line (with the remaining wslpath: response being the bug we're trying to fix). So it looks like that stderr output is the cause.

 ~/p/foo $ wslpath /home/platestacker/projects/foo
wslpath: /home/platestacker/projects/foo
wslpath: /home/platestacker/projects/foo
 !  ~/p/foo $ wslpath /home/platestacker/projects/foo 2>/dev/null
wslpath: /home/platestacker/projects/foo
 !  ~/p/foo $

this is a super long-winded way to say I think we just need to add -w to line 130. Tested that and it seems to stop the spam.

PR: https://github.com/oh-my-fish/theme-bobthefish/pull/361

millimoose commented 3 months ago

@BlakeBLuther Oh I think I know what happened then, I'm guessing I was using a different implementation of wslpath than the one from wslu which seems to have won as the de facto standard.

bobthecow commented 3 months ago

@millimoose does -w work with the version you have? Is there a way to make it work with both flavors? Or at least detect when we have the wrong one?

justindthomas commented 3 months ago

For me, -w fixes the wslpath spam, but the git display remains broken. Commenting out the line altogether as suggested in https://github.com/oh-my-fish/theme-bobthefish/issues/360#issuecomment-1998382348, brings the prompt back to what I'm used to seeing.

This is using Debian on WSL.

bobthecow commented 3 months ago

Okay, I've removed it in 83bf89d10c259680fcf6baab052ff2808ac8e652. Let's try to figure out a fix that works for everyone?