search

Gabriella439 / turtle

Shell programming, Haskell style
BSD 3-Clause "New" or "Revised" License
943 stars 90 forks source link

Migrate to use OSPath instead of FilePath #455

Open cloudyluna opened 6 months ago

cloudyluna commented 6 months ago

Hello! Let me start by saying thanks for this great library. I learned a lot by using it so far as a Haskell beginner. :smiley:

Anyways, I noticed that filepath package recommends to use OSPath instead of FilePath.

Since I believe that turtle is largely being used as a shell scripting alternative (including in cross-platform manner) and which means, may involve a lot of file and directory manipulations, I reckon it's probably better in long term to adopt OSPath?

I was inspired to open this issue after reading this post on Discourse.

Thanks!

Gabriella439 commented 6 months ago

The main issue I see is that OsPath doesn't have an IsString instance. Other than that I'd be fine with migrating to it

hasufell commented 2 months ago

The main issue I see is that OsPath doesn't have an IsString instance. Other than that I'd be fine with migrating to it

It doesn't have one, because we can't write a sound instance.

srghma commented 2 months ago

maybe switch to https://hackage.haskell.org/package/path ?

https://github.com/Gabriella439/turtle/issues/254#issuecomment-324194117

hasufell commented 2 months ago

maybe switch to https://hackage.haskell.org/package/path ?

#254 (comment)

It has the same issue and it's even worse, because you can have 4 different types:

Which one is it gonna be when you write "foo"? You could distinguish via trailing slash, but that's fairly finicky. I guess for turtle that could make sense... so just write an orphan instance.

Wrt OsPath the issue is more nuanced (note that path now also has an OsPath variant: https://hackage.haskell.org/package/path-0.9.6/docs/OsPath.html), because you have to assume an encoding. Again, doing that generically for IsString is just incredibly bad API, so we won't do that. For this library, it can make sense to assume utf-8/16. So again: an orphan instance seems fine. os-string/filepath won't add one, so you don't have to worry about that.

srghma commented 2 months ago

I dont know why we need IsString at all

Gabriella439 commented 2 months ago

One reason why I insist on an IsString instance is that turtle is supposed to be ergonomic to use from within a ghci REPL

srghma commented 2 months ago

I think IsString is possible

Path Rel Dir - ./foo/ Path Abs Dir - /foo/ Path Rel File - ./foo.txt Path Abs File - /foo.txt

hasufell commented 2 months ago

I think IsString is possible

Path Rel Dir - ./foo/ Path Abs Dir - /foo/ Path Rel File - ./foo.txt Path Abs File - /foo.txt

You're aware parsing can fail, right? E.g. /foo/../bar is not permitted by the path library.

Since IsString can't express failure, what are you going to do?

srghma commented 2 months ago

Right, I didnt think about it @Gabriella439 maybe better use TemplateHaskell?