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

windows: set default path separator to '/' in MSYS #2562

Closed
wants to merge 1 commit into from

Conversation

EJammy
Copy link

@EJammy EJammy commented Jul 19, 2023

Adapted from sharkdp/fd#730
Also see sharkdp/fd#537 (comment)

This removes the need to use --path-separator for MSYS and MSYS2 environments (such as Git Bash), as per #275, but not for Cygwin (see linked pr and comment).

Not sure if this is welcomed, since it is handling a special case for specific environment. But I think this is a good improvement as it aligns with the behavior of fd and removes error when running commands such as rg foobar --files-with-matches | xargs without --path-separator.

@BurntSushi
Copy link
Owner

In terms of implementation, I think this logic should probably live in ripgrep proper and not inside grep-printer.

In terms of idea, I'm less sure about this. It seems like it's easy enough to work-around by defining an alias or a config file to set the default path separator yourself. It doesn't feel right to be doing this automatically behind your back. And this is a very subtle (IMO) change in behavior and it's not clear to me what the impact of it would be.

@EJammy
Copy link
Author

EJammy commented Jul 20, 2023

I'm aware of the workaround. I proposed this change because it took me a while to figure out the error after piping the results of rg --files-with-matches, while most tools under MSYS, like fd and git, "just works". I installed ripgrep through MSYS2 with pacman -S mingw-w64-x86_64-ripgrep, so I expected it to work well under that environment.

However, this method of installation is not documented, so I figured it might not have the best support. And I have to agree when I discover this solution it also felt a bit wrong to me too. Might be best to just leave it for the user to configure and improve documentation (#1667).

@BurntSushi
Copy link
Owner

Apologies for my potential ignorance here, but could you say more about the relationship between cygwin and MSYS? In my brain, I seem to remember them being connected or the same in some way, but I find myself unable to articulate precisely their relationship.

@EJammy
Copy link
Author

EJammy commented Jul 21, 2023

I'm not an expert either. I've only recently started using MSYS2 to work in windows and have not touched cygwin before.

But from my understanding, MSYS is built on top of cygwin and provides tools to build and run native windows software. It also has a package manager (pacman). It's also worth noting that git bash (from git for windows) is based on MSYS2, so when people mention git bash it's just repackaged MSYS2.

There's more info on their home page: https://www.msys2.org/
Some additional differences: https://www.msys2.org/wiki/How-does-MSYS2-differ-from-Cygwin/

@BurntSushi
Copy link
Owner

All righty, thanks. I think I'm going to close this then.

I'm not necessarily 100% opposed to something like this, but I just don't have a ton of visibility into its impact. And in general, I've found the whole cygwin/MSYS2 environment to be pretty difficult to fit into. The fundamental problem is that there is no cygwin Rust target, so Rust programs in cygwin don't get linked with the cygwin POSIX emulation layer (or whatever it's called). They stay native Windows programs. So it's best to think about ripgrep as always a native Windows program and not something that is built for cygwin.

@BurntSushi BurntSushi closed this Aug 29, 2023
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.

None yet

2 participants