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

set default path separator to '/' in MSYS #730

Merged
merged 1 commit into from
Feb 15, 2021

Conversation

aswild
Copy link
Contributor

@aswild aswild commented Feb 14, 2021

MSYS and MSYS2 environments (such as Git Bash) have a UNIX like
filesystem which uses '/' as the path separator rather than '', but
Rust doesn't know about this by default.

On Windows, check the MSYSTEM environment variable and set the default
value of the --path-separator option to '/' for convenience.

There is no similar detection of Cygwin because there seems to be no way
for Rust (and any native Win32) programs to detect that they're being
called from a Cygwin environment. Cygwin users can use a shell
alias/function/script to wrap fd.

Fixes: #537

@aswild aswild force-pushed the pr/msys-default-path-separator branch from b7517f1 to 454f411 Compare February 14, 2021 23:58
@aswild
Copy link
Contributor Author

aswild commented Feb 14, 2021

bah, MSRV strikes again - no Result::as_deref() until 1.47. Should be fixed now

src/main.rs Outdated
} else {
None
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we could move this function to src/filesystem.rs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@sharkdp
Copy link
Owner

sharkdp commented Feb 15, 2021

Thank you very much!

This looks great. I added a minor inline comment.

bah, MSRV strikes again - no Result::as_deref() until 1.47. Should be fixed now

I'm usually also very open to bumping the MSRV, but it doesn't seem to bad here to still support 1.40.

MSYS and MSYS2 environments (such as Git Bash) have a UNIX like
filesystem which uses '/' as the path separator rather than '\', but
Rust doesn't know about this by default.

On Windows, check the MSYSTEM environment variable and set the default
value of the --path-separator option to '/' for convenience.

There is no similar detection of Cygwin because there seems to be no way
for Rust (and any native Win32) programs to detect that they're being
called from a Cygwin environment. Cygwin users can use a shell
alias/function/script to wrap fd.

Fixes: sharkdp#537
@aswild aswild force-pushed the pr/msys-default-path-separator branch from 454f411 to c7e5873 Compare February 15, 2021 17:36
@aswild
Copy link
Contributor Author

aswild commented Feb 15, 2021

I'm usually also very open to bumping the MSRV, but it doesn't seem to bad here to still support 1.40.

Yeah, it's no trouble to support 1.40 in this case, I'm just consistently surprised at how new of Rust features I use without even noticing (until CI fails)

@sharkdp sharkdp merged commit 1a3615d into sharkdp:master Feb 15, 2021
@sharkdp
Copy link
Owner

sharkdp commented Feb 15, 2021

until CI fails

If you want, rustup/cargo make it rather easy to have multiple Rust chains in parallel:

rustup toolchain install 1.40

and then:

cargo +1.40 build/test

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.

Option --path-separator failing on MSys (Git Bash)
2 participants