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

AbstractFilePath support #401

Open
hasufell opened this issue Mar 23, 2022 · 3 comments
Open

AbstractFilePath support #401

hasufell opened this issue Mar 23, 2022 · 3 comments
Labels
question Further information is requested reexport Reexport something new

Comments

@hasufell
Copy link

haskell/filepath#103

AFPP will hit filepath sometime soon. What remains to be done is adding support for it in unix and Win32 (I have patches for those).

It will not be included in base, since the earliest GHC version that could support it is 9.6.

What I'm looking for now is an alternative Prelude that is willing to support AFPP, so I can (aggressively) market said prelude in filepath.

Any such prelude would very likely have to additionally depend on unix/Win32 and add functions like openFile :: AbstractFilePath -> IO Handle, withOpenFile, etc.

I have my own file io library that has preliminary patches for AFPP and could be salvaged for such efforts: https://github.com/hasufell/hpath/tree/abstract-filepath/hpath-directory

Any opinions?

@chshersh chshersh pinned this issue Apr 4, 2022
@chshersh chshersh added question Further information is requested reexport Reexport something new labels Apr 14, 2022
@chshersh
Copy link
Contributor

Hi @hasufell 👋🏻

One of the initial motivations behind relude is exactly the ability to introduce changes easier and quicker than to base and be able to experiment with them to understand what fits standard library better. In that case, AFPP seems like a good candidate for for achieving this goal 👍🏻 But only if this is an approved approach that is definitely going to be implemented in core libraries and officially supported (and not just another alternative approach to the "filepath" problem in Hasklel).

I'm out of the loop for the state of AFPP. Could you provide any links confirming that the proposal is accepted?


As for unix and Win32 dependencies, adding them to relude deps is fine as they are both boot libraries. Ideally, I'd like to have all deps as boot libraries (including the libs implementing AFPP) but I believe this will be the case anyway if filepath is going to depend on them.


I see the implementation of AFPP in relude as a way to help people transition to AFPP earlier. I'm not sure what benefits they have from this (better performance, more type-safety, simpler API, earlier cross-platform support, etc.) but you can provide more motivation here.

In that case, you can try to implement and contribute the new Relude.Extra.IO or Relude.Extra.FilePath module with the suggested new API of AFPP. And if everything is good, we can further deprecate old FilePath -> ... functions in relude and recommend using the new interface to provide a smooth migration plan.

@hasufell
Copy link
Author

hasufell commented Apr 15, 2022

I'm out of the loop for the state of AFPP. Could you provide any links confirming that the proposal is accepted?

Yes, CLC has approved AFPP (I don't remember if that has been done publicly, but I have proof). Further, since I'm maintainer of filepath now, this will definitely land either way.

The major outstanding non-filepath PRs are:

As for unix and Win32 dependencies, adding them to relude deps is fine as they are both boot libraries. Ideally, I'd like to have all deps as boot libraries (including the libs implementing AFPP) but I believe this will be the case anyway if filepath is going to depend on them.

There's the small companion package for openFile and friends https://github.com/hasufell/file-io that won't become a boot library, but can easily be inlined

I haven't created a PR for directory package yet as I'm personally not that much interested in it and instead plan to get my own library up to speed: https://github.com/hasufell/hpath
But it's possible.

I see the implementation of AFPP in relude as a way to help people transition to AFPP earlier. I'm not sure what benefits they have from this (better performance, more type-safety, simpler API, earlier cross-platform support, etc.) but you can provide more motivation here.

The main motivation is to preserve encoding on unix. Once you go from CString to String, the information is simply gone. That can cause subtle bugs. The new approach also may be more performant, since it doesn't require encoding/decoding in most cases and we use a proper byte array instead of a linked list. But there aren't any benchmarks for it. These are the advantages compared to String. The advantage compared to ByteString is that we don't have memory fragmentation, because we use ShortByteString under the hood (unpinned byte array).

@chshersh
Copy link
Contributor

This looks good to me 👍🏻

@hasufell Feel free to contribute the corresponding Relude.Extra.* module with the support of AFPP in relude when the time comes. I imagine this won't be a huge file. Maybe reexport of a few functions, am I right? You can provide more details on the AFPP support in relude and how you envision this before actually implementing it 🙂

relude doesn't reexport neither filepath nor directory so we don't need to wait for the directory to catch up.

I believe depending on file-io is also fine here. I don't think inlining this library is a good idea (because I personally don't want to maintain this complicated code). And it's a rather small library so depending on it should be fine 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested reexport Reexport something new
Projects
None yet
Development

No branches or pull requests

2 participants