junegunn / fzf

:cherry_blossom: A command-line fuzzy finder
https://junegunn.github.io/fzf/
MIT License
62.33k stars 2.35k forks source link

Update charlievieth/fastwalk to use forward-slashes on WSL and MSYS #3907

Closed charlievieth closed 1 week ago

charlievieth commented 2 weeks ago

This commit changes FZF to enforce that all paths are joined with forward-slashes when running on WSL (Windows Subsystem for Linux) even when the FZF binary was compiled for Windows.

Update: github.com/charlievieth/fastwalk to v1.0.5 Fixes: https://github.com/junegunn/fzf/issues/3859

charlievieth commented 2 weeks ago

NOTE: This is just a draft and is incompatible with the -walker-path-sep option since (it only allows the option to force forward slashes, but does auto-detect when they're needed on WSL see: DefaultToSlash).

junegunn commented 2 weeks ago

Thanks, I wish we could avoid adding --walker-path-sep.

Defaulting to / on WSL sounds great. But we may also do that on MSYS2.

Related: https://github.com/sharkdp/fd/pull/730

junegunn commented 1 week ago

incompatible with the -walker-path-sep option since

With this and #3909, we can safely remove --walker-path-sep.

junegunn commented 1 week ago

So I removed --walker-path-sep, updated the PR branch, applied the latest version of fastwalk (go get github.com/charlievieth/fastwalk@master), tested it on MSYS2, but it wasn't working as expected.

Turned out that filepath.Clean(path) is replacing / back to \ 😢

https://cs.opensource.google/go/go/+/refs/tags/go1.20.13:src/path/filepath/path.go;l=166

junegunn commented 1 week ago

We probably don't need the whole filepath.Clean(path). We should be fine, even if we just remove the ./ or .\.

junegunn commented 1 week ago

9b30ec559c908b83142b8f6d5ffad5524df427b3 removes filepath.Clean. It does not perfectly clean up fzf --walker-root ./././../././, but I don't care.

junegunn commented 1 week ago

Merged, thanks!