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

Should OsPath be (public) newtype over OsString anyway? #165

Open
hasufell opened this issue Aug 2, 2022 · 3 comments
Open

Should OsPath be (public) newtype over OsString anyway? #165

hasufell opened this issue Aug 2, 2022 · 3 comments

Comments

@hasufell
Copy link
Member

hasufell commented Aug 2, 2022

In GitLab by @phadej on Aug 2, 2022, 19:27

I'm thinking this from the POV if there were "right" IsString type-class.

Currently osp and osstr quasiquoters are different, but as type OsPath = OsString we cannot do type-directed resolution.

@hasufell
Copy link
Member Author

hasufell commented Aug 2, 2022

In GitLab by @maerwald on Aug 2, 2022, 20:33

I'm not too sure either.

Because although the quasiquoters are different, the OsPath type doesn't maintain the isValid invariant. Because that's actually not trivial and requires to make trade off decisions that I feel are not in scope for a low level library (compare with path/hpath/strong-path etc).

So my intuition is that a newtype just for the sake of different instances, but without additional guarantees, is kinda confusing.

@hasufell
Copy link
Member Author

hasufell commented Aug 3, 2022

In GitLab by @phadej on Aug 3, 2022, 18:39

I see. So it's "not wrong" to use osstr to create OsPath as far as filepath is concerned, it is just so you probably would like to catch a bit more errors.

That is an interesting point. E.g. if we could have a good IsString Day instance, so we could write "2022-08-03" to create a day, but there might be use cases where you want to create dates only before certain day, or only in 2000+ etc. I have no idea what a syntax for such literals would look like, but I see a value for it: OsPath would be one such subset of OsString. (I consider quasiquoters just a bit too heavy syntactically, also untyped).

If you agree, we can close this issue.

@hasufell
Copy link
Member Author

hasufell commented Aug 3, 2022

In GitLab by @maerwald on Aug 3, 2022, 19:25

Well, I think OsPath being a newtype would be interesting, but there are two problems:

  1. isValid on windows is not too well defined. The current function is not exhaustive so to speak. Additionally, you can absolutely use invalid windows paths with the \\?\ prefix. You'll get mixed results, but it's possible.
  2. Maintaining validity may now require functions to return Maybe instead of empty string, path separator etc, so that would be a huge breaking change to the api. There's also some odd corner cases that are discussed here with such APIs: Use GitHub actions for CI commercialhaskell/path#163 (comment)

That's why I can't come up with a simple design that doesn't make opinionated decisions.

Otherwise, a non-class approach with top-level functions seems easier. But that's not compatible with our OverloadedStrings approach (if only we could provide tags or whatnot like in python).

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

No branches or pull requests

1 participant