Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Jul 8, 2024

Conversation

charlievieth
Copy link
Contributor

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: #3859

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:  junegunn#3859
@charlievieth
Copy link
Contributor Author

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
Copy link
Owner

junegunn commented Jul 3, 2024

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: sharkdp/fd#730

@charlievieth charlievieth changed the title fzf: update charlievieth/fastwahk and use forward-slashes on WSL fzf: update charlievieth/fastwalk and use forward-slashes on WSL Jul 3, 2024
src/reader.go Outdated Show resolved Hide resolved
@junegunn
Copy link
Owner

junegunn commented Jul 4, 2024

incompatible with the -walker-path-sep option since

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

junegunn added a commit that referenced this pull request Jul 5, 2024
@junegunn junegunn changed the title fzf: update charlievieth/fastwalk and use forward-slashes on WSL fzf: update charlievieth/fastwalk and use forward-slashes on WSL and MSYS Jul 5, 2024
@junegunn junegunn changed the title fzf: update charlievieth/fastwalk and use forward-slashes on WSL and MSYS Update charlievieth/fastwalk to use forward-slashes on WSL and MSYS Jul 5, 2024
src/reader.go Outdated Show resolved Hide resolved
@junegunn junegunn marked this pull request as ready for review July 5, 2024 11:20
@junegunn
Copy link
Owner

junegunn commented Jul 5, 2024

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
Copy link
Owner

junegunn commented Jul 5, 2024

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

@junegunn
Copy link
Owner

junegunn commented Jul 5, 2024

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

@junegunn junegunn merged commit 2dbc874 into junegunn:master Jul 8, 2024
5 checks passed
@junegunn
Copy link
Owner

junegunn commented Jul 8, 2024

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing directory slashes when completion chosen
2 participants