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

Can't parse windows style absolute paths #19

Open
danyx23 opened this issue Jul 24, 2016 · 8 comments
Open

Can't parse windows style absolute paths #19

danyx23 opened this issue Jul 24, 2016 · 8 comments
Labels
type: bug Something that should function correctly isn't.

Comments

@danyx23
Copy link

danyx23 commented Jul 24, 2016

I just naively tried to use this library under windows because it is advertised as "platform independent paths" and ran into trouble :). Currently, the library doesn't have an Escaper for windows directory seperators (backslash instead of forward slash) and can't parse absolute paths as used on the windows platform ("C:\Users").

There are more cases that would be nice to handle, most importantly network share paths ("\server\share\directory"), and some others that are more questionable (e.g. long path urls that start with "?").

Are you interested in extending this library to handle windows paths as well? If so I could try to write up a plan on how to add windows path support to purescript-pathy.

@garyb
Copy link
Member

garyb commented Jul 24, 2016

At the moment the library doesn't use the escapers at all actually, but #18 will fix that.

I'll let @jdegoes comment on the rest 😉

@jdegoes
Copy link
Contributor

jdegoes commented Jul 24, 2016

I am interested in the following improvements:

  1. Use escapers, and refine the notion of an escaper so it's more robust (Some tweaks and updates for the next release #18 et al).
  2. Introduce escapers for Windows, OS X, Linux.
  3. Introduce a current path node, e.g., ., so it's possible to roundtrip paths like ../././../foo/bar/./baz/...
  4. Introduce a powerful fold method and use that to implement all the "unsafe" (type parameter-changing) operations.
  5. Change the API so all absolute paths are sandboxed (which means returning Maybe in a few places); or possibly, delete the notion of sandboxing but introduce equivalent functionality.

I think there's a place for a function which can parse a Windows path into Pathy's structure, e.g. /C/Users/, and of course a Windows escaper and maybe a renderer. Would that be sufficient, or would you want to hard code some notion of volume? I don't think the latter would work well in Pathy, it's not designed to have OS-specific components like that.

@danyx23
Copy link
Author

danyx23 commented Jul 24, 2016

Thanks for your replies! Purely from a users perspective, I think it would be great if I could use pathy to parse paths the user gives me, manipulate them with pathy and then use the result with the purescript-node-fs functions to actually manipulate files and directories. Since a javascript node program will be used on both posix and windows environments, that would mean handling windows paths as well (e.g. if they are passed by the user as a command line argument).

It is possible to get pretty far by parsing "C:\Users" into the internal representation /C/Users/ and then doing the reverse transform when converting back to a string on windows (just passing "/C/Users/" to the node runtime on windows does not work). One of the annoying details on windows is that volumes act as independent roots, i.e. unlike Posix paths there is no guarantee that you can create a relative path between two absolute paths on windows.

The [node path api](#! https://nodejs.org/api/path.html) behaves different depending on the environment it is in which I think is a pretty undesirable trait - for one thing it makes platform independent testing difficult.

I haven't really looked at the proposed code for version 2 of this library that will use Escapers, but I think the idea to be able to specify the behaviour with an Escaper and being able to get a default Escaper depending on the runtime environment would be a pretty nice way to deal with the problem.

Do you think this is an idea worth pursuing?

@jdegoes
Copy link
Contributor

jdegoes commented Jul 24, 2016

The only part I think doesn't belong in pathy is "get a default Escaper depending on the runtime environment". Currently, pathy is a 100% pure project, could be used by any backend, and advertises itself as being platform-independent. I think we would want to create side-projects like purescript-pathy-os or something to hold node-like dependencies. So while I'd love to have different escapers and such that encode knowledge of paths on different systems, I'd still like this to be 100% pure code with no Eff or dependency on anything in the node (or other platform) ecosystem.

Seem reasonable?

@danyx23
Copy link
Author

danyx23 commented Jul 25, 2016

Yes, that sounds like a great solution.

@garyb garyb marked this as a duplicate of #29 Jul 17, 2017
@garyb garyb added the type: bug Something that should function correctly isn't. label Jul 17, 2017
@safareli safareli mentioned this issue Feb 16, 2018
@stkb
Copy link

stkb commented Jul 1, 2019

Noticed there still isn't a windowsParser in the library. Does it just need someone to write it or is there something else? Even though the library doesn't have a concept of a "named root", it seems to be already handled by just having the drive letter as the first part of the path.

@garyb
Copy link
Member

garyb commented Jul 1, 2019

Yeah, a printer/parser for Windows still needs writing. I think treating the drive letter as the first part of the path, and just catching that in the printer and adding the colon as necessary would be an option there.

It wouldn't support all windows paths, since there's the UNC \\foo\... style paths and all sorts of other things too, but would at least work for basic filesystem stuff!

@srghma
Copy link
Member

srghma commented Oct 7, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something that should function correctly isn't.
Development

No branches or pull requests

5 participants