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

In cygwin, fd output displays backslash instead of forward slash #153

Open
rajeshduggal opened this issue Oct 28, 2017 · 26 comments

Comments

Projects
None yet
10 participants
@rajeshduggal
Copy link

commented Oct 28, 2017

No description provided.

@sharkdp

This comment has been minimized.

Copy link
Owner

commented Nov 1, 2017

Hi, thank you for your feedback.

Could you please give a little bit more details (fd version, operating system, etc.)?

Why do you think fd should output / as directory separator instead of \? Isn't cygwin running on Windows?

@sharkdp sharkdp added the question label Nov 1, 2017

@rajeshduggal

This comment has been minimized.

Copy link
Author

commented Nov 2, 2017

Most tools in cygwin output a / separator, but fd uses \ instead...
e.g..
$ uname -a
CYGWIN_NT-6.1 TDL00885482 2.9.0(0.318/5/3) 2017-09-12 10:18 x86_64 Cygwin
$ fd --version
fd 4.0.0
$ ls -1 dax/*
dax/oswald.txt
$ tree -if dax
dax
dax/oswald.txt
0 directories, 1 files
$ find ./dax
./dax
./dax/oswald.txt
$ fd '.*txt' ./dax
dax\oswald.txt

@tmccombs

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2017

was fd compiled with cygwin, or as a windows binary?

@rajeshduggal

This comment has been minimized.

Copy link
Author

commented Nov 6, 2017

https://github.com/sharkdp/fd/releases/download/v5.0.0/fd-v5.0.0-x86_64-pc-windows-gnu.zip

So I guess it's understandable that it's output shows windows backslash convention.
(fyi. There's an interesting discussion thread on a similar cygwin path issue for a different project here.. BurntSushi/ripgrep#269 )

@sharkdp

This comment has been minimized.

Copy link
Owner

commented Nov 8, 2017

We could potentially change this, but I'm not sure if / would always be preferable over \. Do all tools on Windows support /? Is there any drawback from using \ or is this more of an aesthetic thing?

It says fd v4.0.0 in your output but you downloaded v5.0.0? Could you please make sure that you are on the latest verstion? There have been a few changes to path handling (on Windows) in 5.0.0 - although this will likely not fix this issue.

@Doxterpepper

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2017

I was actually curious about this. When compiled on windows fd will always replace forward slashes with back slashes. Doesn't PathBuf already format the slashes depending on the platform?

@Doxterpepper

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2017

@sharkdp, it looks like fd is using Path::from which will convert the path to the platform specific convention. We could maybe figure out a way to determine if we're running from cygwin when we print to the console and then change the back slashes to forward slashes then.

As a side note from what I can tell let path = path.replace('/', "\\") is unnecessary because of PathBuf.

@rajeshduggal

This comment has been minimized.

Copy link
Author

commented Nov 10, 2017

I was running fd v4.0.0 and then noticed there was a newer version and so I installed that.. hence the discrepancy.

I don't have rust installed.
https://doc.rust-lang.org/std/path/constant.MAIN_SEPARATOR.html
Anyone know if it returns \ versus / when it's run on cygwin verses dos prompt?

sharkdp added a commit that referenced this issue Nov 10, 2017

sharkdp added a commit that referenced this issue Nov 10, 2017

sharkdp added a commit that referenced this issue Nov 11, 2017

@sharkdp

This comment has been minimized.

Copy link
Owner

commented Nov 20, 2017

It looks like it's hard-coded for the different operating systems:

https://github.com/rust-lang/rust/blob/94ede93467beb4ac17956845607a7e8b226dda02/src/libstd/sys/windows/path.rs#L106

https://github.com/rust-lang/rust/blob/144af3e97aa30feba3d36a98ac04c0f1b2bc0bea/src/libstd/sys/unix/path.rs#L29

If I may, I'd like to repeat my question from above:

We could potentially change this, but I'm not sure if / would always be preferable over . Do all tools on Windows support /? Is there any drawback from using \ or is this more of an aesthetic thing?

@ghuls

This comment has been minimized.

Copy link

commented Nov 22, 2017

@Doxterpepper Without Cygwin path translation, your patch might complicate things.

fd will not be able to use Cygwin only paths like /cygdrive and /home while now it might look to the end user that this is the case now, also fd '*.txt' C:\ will use forward slashes, now, which would be strange to me.

@Doxterpepper

This comment has been minimized.

Copy link
Contributor

commented Nov 23, 2017

@ghuls, find also prints forward slashes when you search C:\\, in fact it prints an odd C:\/some/path. Also I think in either case users may think /cygdrive is a usable path, I don't think just printing the correct slashes for the environment is going to change the users perceptions on what directories are usable and what directories are not.

@Doxterpepper

This comment has been minimized.

Copy link
Contributor

commented Nov 23, 2017

@sharkdp, Cygwin is supposed to be a posix compliment environment so the forward slashes are always going to be preferred in any application running in Cygwin. Our options are to either not do anything and let the user handle it with cygpath or something else, or to check for a environment variable such as HOME which is normally only defined in a Unix like environment to determine if we should use forward or back slashes. I think my patch may not be preferred now that I think about it because if the user for some reason did set HOME=something fd will print forward slashes instead of back slashes in CMD.

@rajeshduggal

This comment has been minimized.

Copy link
Author

commented Dec 8, 2017

IMHO, (Selfishly) I like the idea of having basic default fd functionality being a direct drop-in replacement for basic find functionality. So by default it should mimic cygwin's find output.

@danielpclark

This comment has been minimized.

Copy link

commented Dec 8, 2017

For the record Windows officially supports forward slash.

According to Microsoft:

File I/O functions in the Windows API convert / to \ as part of converting the name to an NT-style name, except when using the \?\ prefix as detailed in the following sections.

Source: https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx

@gene-pavlovsky

This comment has been minimized.

Copy link

commented Dec 14, 2017

I'm a Cygwin user, and for example the reason I'm using the_silver_searcher instead of ripgrep is that there is not Cygwin build of the latter.

I just found out about fd and it sounds very neat, I'd love to be able to use it. In my case, I don't care for backslashes under any circumstances, so I'd like to have a Cygwin specific path option for fd. A note: prefix for Windows drives is /cygdrive/ by default, but it can be user-defined by editing Cygwin's /etc/fstab, e.g. I'm using /mnt, therefore it would be not correct if a tool would just output /cygdrive/c/path/to/result without thinking. Another consideration is the home dir - it also depends on where Cygwin is installed, so /home can correspond to C:\cygwin\home or C:\cygwin64\home or elsewhere.
For fully correct Cygwin support all this stuff has to be considered, it would be best to have Cygwin-specific build that links to cygwin1.dll and uses cygwin_create_path to do the dirty work.
But if this is too much, just using / would be mostly ok, as long as the user is careful to use relative paths.

P.S. There is another popular (rather minimal) UNIX-like environment Git bash, part of Git for Windows, which uses paths like /c/path/to/result.

@sharkdp

This comment has been minimized.

Copy link
Owner

commented Dec 18, 2017

Ok, it looks like a cygwin-specific build could be an option. I would like to avoid pulling in more dependencies (even optional), but if this can be configured using compile-time flags, it sounds like a good option to me.

I will likely not find the time to work on this, but if someone wants to contribute, I'm happy to assist / discuss.

@sharkdp sharkdp added easy help wanted idea and removed question labels Dec 18, 2017

@gene-pavlovsky

This comment has been minimized.

Copy link

commented Dec 18, 2017

I think I could handle it (although I don't know Rust). What is the best channel for discussion?

@sharkdp

This comment has been minimized.

Copy link
Owner

commented Dec 19, 2017

Great!

You could open a ("work-in-progress") pull request and we can discuss there or we can just use this ticket. Whatever you prefer.

@tmccombs

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2017

I'm a little surprised there isn't a x86_64-pc-windows-cygwin target for rustc.

@cyrossignol

This comment has been minimized.

Copy link

commented Dec 21, 2017

For what it's worth, I sometimes use something similar to the following in Cygwin/MSYS flavors as a workaround:

fd() {
    command fd "$@" | cygpath --ignore --file -
}

...defined in .bashrc or equivalent. It streams the output through cygpath to convert the paths to Unix-style.

Considering what cygpath does, it's pretty fast, but it would be nice if fd handled the conversion to reduce the overhead. Many Cygwin programs do understand Windows-style paths, so this isn't always necessary.

@rajeshduggal

This comment has been minimized.

Copy link
Author

commented Dec 21, 2017

fyi: ag shows the expected slash when run in cygwin too. https://github.com/ggreer/the_silver_searcher/search?utf8=%E2%9C%93&q=cygwin&type=

@jedahan

This comment has been minimized.

Copy link

commented Mar 11, 2018

Is there an appropriate working group we could share this issue with?

@tmccombs

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2018

the CLI working group seems appropriate. https://internals.rust-lang.org/t/announcing-the-cli-working-group/6872

@ErichDonGubler

This comment has been minimized.

Copy link

commented Jun 13, 2018

ripgrep actually uses an option to alleviate a lot of the pain associated with this particular portability problem -- have you considered adding something like path-separator?

@sharkdp

This comment has been minimized.

Copy link
Owner

commented Jun 15, 2018

@ErichDonGubler Interesting. II have not considered it, but I'm open to adding this to fd, if it solves (or alleviates) this particular issue here.

@ErichDonGubler

This comment has been minimized.

Copy link

commented Jun 15, 2018

@sharkdp: I know that @BurntSushi actually decided to stick a fork in finding a "real" solution for the problem because it was so time-intensive. Using a shim above like @cyrossignol is something I also do for my MSYS2 setup -- while not totally palatable, it at least makes things bearable and is a good partial solution to this problem.

The crux of the issue here is that the Cygwin/MSYS2 environment sidesteps the issue of portability like Rust needs to deal with here by creating a different compilation environment. Many C/C++ solutions can straightforwardly solve this problem by recompiling with the MinGW version of gcc and using macros to expose whether code is being compiled against vanilla Windows or MinGW. Rust has no such luxury, however!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.