Repository: junegunn/fzf (language: go)

Issue:
Missing directory slashes when completion chosen
### Checklist

- [X] I have read through the manual page (`man fzf`)
- [X] I have searched through the existing issues
- [ ] For bug reports, I have checked if the bug is reproducible in the latest version of fzf

### Output of `fzf --version`

0.53.0 (c4a9ccd)

### OS

- [ ] Linux
- [ ] macOS
- [X] Windows
- [ ] Etc.

### Shell

- [ ] bash
- [X] zsh
- [ ] fish

### Problem / Steps to reproduce

Thanks for the fab tool.

For a new windows install I'm working on I am getting the backslashes that separate directories all go missing when I hit enter to choose an entry which then means the path is corrupted.

I have done a load of searching and can't find any mention of this being an issue anywhere or what it might be. I don't know enough about how fzf works to guess further so any ideas welcome.

Begin a completion with `file **<tab>`:
![image](https://github.com/junegunn/fzf/assets/19378/7f2235c6-44c9-4c30-94a9-935495e2ee65)

Hit `enter` to choose an item in a subfolder and all the slashes are missing:
![image](https://github.com/junegunn/fzf/assets/19378/de10d73a-7ae0-472f-9b1b-bb6602cff5df)

Curiously if I call `fzf` directly and then search it works fine:

![image](https://github.com/junegunn/fzf/assets/19378/76c6ed6c-7f3a-4847-a690-76f8c50f1780)

![image](https://github.com/junegunn/fzf/assets/19378/82e5e469-66c0-40bd-8687-3dac35fcc93d)

This is my dotmatrix config for the windows machine (windows branch) at time of writing https://github.com/timabell/dotmatrix/blob/0c0de5b7107de4a43c22f1f6c727b2537ffc1056/.zshrc#L107


Discussion / hints:
Is this WSL? Looks like you're running an fzf binary for Windows that uses backslash as the file separator. How did you install fzf?
You should be using Linux binary on WSL. It works for me fine.

<img width="755" alt="image" src="https://github.com/junegunn/fzf/assets/700826/71fbb9ff-b4cf-4b07-89fa-394685f8421f">

Please comment if you still have the problem with a linux binary.
Hi, thanks for responding, this is not WSL, this is zsh installed directly on windows, being used in windows terminal.

Yes this is still an issue.

I am unable to use WSL because it's a Virtual Box windows image, and for some reason I can't get the nested virtualisation to work.

I think I installed it with the shallow clone as per the instructions - https://github.com/timabell/dotmatrix/blob/2e554b3093b4d0f8c53f8f2730f0aef63c9e4774/software/fzf.sh#L5-L6 - let me know if you want me to check anything on the windows machine to add more details.
Oh, so does zsh transform the path separators on Windows so you can access paths in Unix style? If that is the case, the Window binary of fzf needs a way to use forward-slashes instead.

Related:
* https://github.com/BurntSushi/ripgrep/issues/1667#issuecomment-681940449
* https://github.com/sharkdp/fd/issues/428
@charlievieth Hi, would it be possible to add path separator option to your fastwalk library for cases where you want forward slashes on Windows? I guess fzf can replace backslashes with slashes afterwards if that is not possible.
> @charlievieth Hi, would it be possible to add path separator option to your fastwalk library for cases where you want forward slashes on Windows? I guess fzf can replace backslashes with slashes afterwards if that is not possible.

My first thought is that fastwalk is really only concerned about directory traversal and should provide its callback canonical paths so maybe an adapter like the one below would work (which if need be I could add to [fastwalk as an adapter](https://github.com/charlievieth/fastwalk/blob/master/adapters.go)):

```go
// ~120ns for a path like "C:\Users\username\go\src\github.com\charlievieth\fastwalk"
// no-op if the path separator is `/`
func ForwardSlashPaths(walkFn fs.WalkDirFunc) fs.WalkDirFunc {
	return func(path string, d fs.DirEntry, err error) error {
		return walkFn(filepath.ToSlash(path), d, err)
	}
}
```

That said, WSL is a funny thing (Linux, but Windows) and having to convert the path each time does incur some cost so if we think the cost of an adapter is too much then more than happy to add it as an option.
Following up on my last comment: an adapter on the fzf side is best way to immediately fix this issue; I think this should be supported in fastwalk the question is whether it's an adapter (like the one below) or as a config option. I'll think about this and post a new release in the next few days.

```go
func toSlash(path string) string {
	if os.PathSeparator == '/' {
		return path
	}
	i := strings.IndexByte(path, '\\')
	if i < 0 {
		return path
	}
	b := []byte(path)
	for ; i < len(b); i++ {
		if b[i] == '\\' {
			b[i] = '/'
		}
	}
	return unsafe.String(&b[0], len(b))
}

// ~50ns if the PathSeparator != `\`
func ToSlashPaths(walkFn fs.WalkDirFunc) fs.WalkDirFunc {
	return func(path string, d fs.DirEntry, err error) error {
		return walkFn(toSlash(path), d, err)
	}
}
```
Thanks for the quick response!

> an adapter on the fzf side is best way to immediately fix this issue

So if I understood correctly, fzf can replace the separators itself if needed, and it doesn't require a code change in fastwalk. Something like this:

```diff
diff --git a/src/reader.go b/src/reader.go
index f39f47f..3a15ac4 100644
--- a/src/reader.go
+++ b/src/reader.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -254,6 +255,7 @@ func (r *Reader) readFiles(root string, opts walkerOpts, ignores []string) bool
 					}
 				}
 			}
+			path = strings.ReplaceAll(path, `\`, "/")
 			if ((opts.file && !isDir) || (opts.dir && isDir)) && r.pusher([]byte(path)) {
 				atomic.StoreInt32(&r.event, int32(EvtReadNew))
 			}
```



> ```go
> 	b := []byte(path)
> ```

I think`unsafe.StringData` can help here. (if it's okay to modify the bytes in-place)

https://github.com/junegunn/fzf/blob/a06745826a4cba4f578a69258f9def75c59530fc/src/functions.go#L29-L31
bf515a3d3251c54d04cd8e2e9f422cdd3f789fb8 implements `--walker-path-sep`.

Please let me know if it doesn't work as expected.
@junegunn I just cut [v1.0.5](https://github.com/charlievieth/fastwalk/releases/tag/v1.0.5) which includes https://github.com/charlievieth/fastwalk/pull/23 and adds an option for enforcing forward-slashes and adds the [DefaultToSlash](https://pkg.go.dev/github.com/charlievieth/fastwalk#DefaultToSlash) function which detects if we're a Windows executable running under WSL (and enforces forward-slashes if so). This should solve the issue here.

I cut a draft PR https://github.com/junegunn/fzf/pull/3907  that should address this issue. It probably needs to be updated since it's currently incompatible with the `-walker-path-sep` option (it only allows the option to force forward slashes, but does auto-detect when they're needed on WSL see: [DefaultToSlash](https://pkg.go.dev/github.com/charlievieth/fastwalk#DefaultToSlash)).
@charlievieth Thanks for looking into this. I'd also like to avoid adding an option and automatically print the right separator, but @timabell [stated above](https://github.com/junegunn/fzf/issues/3859#issuecomment-2179510062) that he was having the problem not on WSL but on zsh on Windows. I don't have experience with it so I'm not quite sure but it looks like zsh on Windows is doing some path conversion.
@timabell Can you tell us more about your setup? How did you install zsh on Windows? Is it msys?
Hi,

Sorry, I'm not 100% sure how I ended up with this zsh installed. It did quite possibly come with git extensions or msys git. It was a while ago that I set this VM up.

This is the windows terminal config for the zsh profile that I use to launch it:

```json
            {
                "closeOnExit": "always",
                "commandline": "\"%PROGRAMFILES%\\Git\\usr\\bin\\zsh.exe\" --login -i",
                "guid": "{27ed4f8e-4ea3-4dae-b251-0a792cadd5cd}",
                "hidden": false,
                "name": "zsh",
                "startingDirectory": "C:\\dev\\"
            }
```

This is what the zsh says about itself

```sh
╭─User@WinDev2404Eval /c/dev/dotmatrix ‹windows›
╰─$ which zsh
/usr/bin/zsh
╭─User@WinDev2404Eval /c/dev/dotmatrix ‹windows›
╰─$ zsh --version
zsh 5.9 (x86_64-pc-msys)
╭─User@WinDev2404Eval /c/dev/dotmatrix ‹windows›
╰─$ file `which zsh`
/usr/bin/zsh: PE32+ executable (console) x86-64 (stripped to external PDB), for MS Windows, 11 sections
```

How do I test the new version? Currently on 

```
fzf --version
0.53.0 (c4a9ccd)
```

Would above patch fix it or is that only for WSL?
So it's MSYS2.

I just tested these things both on bash and zsh on MSYS2, and realized that they work as expected on bash even though fzf is printing paths with backslashes, because the bash script properly escapes the selected entries.

* `vim <CTRL-T>`
* Select `foo\bar\baz`
* `vim foo\\bar\\baz`. Yes.

However, the escaping part is missing on zsh version, and it becomes `vim foobarbaz`. So we can fix this by properly escaping backslashes on the zsh script.

Using `read -r` instead of `read` is a start.

```diff
diff --git a/shell/completion.zsh b/shell/completion.zsh
index 46103f9..dce74c6 100644
--- a/shell/completion.zsh
+++ b/shell/completion.zsh
@@ -171,7 +171,7 @@ __fzf_generic_path_completion() {
             rest=${FZF_COMPLETION_PATH_OPTS-}
           fi
           __fzf_comprun "$cmd" ${(Q)${(Z+n+)fzf_opts}} -q "$leftover" --walker "$walker" --walker-root="$dir" ${(Q)${(Z+n+)rest}} < /dev/tty
-        fi | while read item; do
+        fi | while read -r item; do
           item="${item%$suffix}$suffix"
           echo -n "${(q)item} "
         done
diff --git a/shell/key-bindings.zsh b/shell/key-bindings.zsh
index 1490595..8cb95cd 100644
--- a/shell/key-bindings.zsh
+++ b/shell/key-bindings.zsh
@@ -52,7 +52,7 @@ __fzf_select() {
   local item
   FZF_DEFAULT_COMMAND=${FZF_CTRL_T_COMMAND:-} \
   FZF_DEFAULT_OPTS=$(__fzf_defaults "--reverse --walker=file,dir,follow,hidden --scheme=path" "${FZF_CTRL_T_OPTS-} -m") \
-  FZF_DEFAULT_OPTS_FILE='' $(__fzfcmd) "$@" < /dev/tty | while read item; do
+  FZF_DEFAULT_OPTS_FILE='' $(__fzfcmd) "$@" < /dev/tty | while read -r item; do
     echo -n "${(q)item} "
   done
   local ret=$?
```

Then it gives `vim foo\bar\baz`. It's an improvement but it's not quite there 😕 

---

You can test this on non-Windows environment by setting

```sh
FZF_CTRL_T_COMMAND="echo 'foo\bar\baz'; echo 'hello\world'"
```

* bash gives `foo\\bar\\baz hello\\world`
* zsh gives `foobarbaz helloworld`

---

Having said that, things will be simpler if fzf prints forward slashes. But not everyone uses the built-in walker, so we still need to fix the script nevertheless.
> You can test this on non-Windows environment by setting
> 
> ```shell
> FZF_CTRL_T_COMMAND="echo 'foo\bar\baz'; echo 'hello\world'"
> ```
> 
> * bash gives `foo\\bar\\baz hello\\world`
> * zsh gives `foobarbaz helloworld`

@LangLangBart Your expertise in zsh will be really helpful here. Would it be possible to make zsh behave the same as bash in this case?

Fix patch:
diff --git a/shell/completion.zsh b/shell/completion.zsh
index 46103f98919..4786bbd36f1 100644
--- a/shell/completion.zsh
+++ b/shell/completion.zsh
@@ -171,9 +171,9 @@ __fzf_generic_path_completion() {
             rest=${FZF_COMPLETION_PATH_OPTS-}
           fi
           __fzf_comprun "$cmd" ${(Q)${(Z+n+)fzf_opts}} -q "$leftover" --walker "$walker" --walker-root="$dir" ${(Q)${(Z+n+)rest}} < /dev/tty
-        fi | while read item; do
+        fi | while read -r item; do
           item="${item%$suffix}$suffix"
-          echo -n "${(q)item} "
+          echo -n -E "${(q)item} "
         done
       )
       matches=${matches% }
diff --git a/shell/key-bindings.zsh b/shell/key-bindings.zsh
index 1490595d25f..eea518995a6 100644
--- a/shell/key-bindings.zsh
+++ b/shell/key-bindings.zsh
@@ -52,8 +52,8 @@ __fzf_select() {
   local item
   FZF_DEFAULT_COMMAND=${FZF_CTRL_T_COMMAND:-} \
   FZF_DEFAULT_OPTS=$(__fzf_defaults "--reverse --walker=file,dir,follow,hidden --scheme=path" "${FZF_CTRL_T_OPTS-} -m") \
-  FZF_DEFAULT_OPTS_FILE='' $(__fzfcmd) "$@" < /dev/tty | while read item; do
-    echo -n "${(q)item} "
+  FZF_DEFAULT_OPTS_FILE='' $(__fzfcmd) "$@" < /dev/tty | while read -r item; do
+    echo -n -E "${(q)item} "
   done
   local ret=$?
   echo


Test patch: