-
Notifications
You must be signed in to change notification settings - Fork 32
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
WIP: Add function stripFilePath
#75
base: master
Are you sure you want to change the base?
Conversation
-- >>> stripFilePath "/app" "/app/Lib/File.hs" | ||
-- Just "Lib/File.hs" | ||
stripFilePath :: FilePath -> FilePath -> Maybe FilePath | ||
stripFilePath "." fp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about:
> stripFilePath "./." "lol"
Nothing
> stripFilePath "./" "lol"
Nothing
I can't figure out what semantics a user would expect here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could take rust as a precedent?
stripPrefix("lol", "./.") -> Err(StripPrefixError(()))
stripPrefix("lol", "./") -> Err(StripPrefixError(()))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so do we want MonadThrow
here maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's in general a possible idea, for the whole API to have those for the case of errors.
-- | ||
-- >>> stripFilePath "/app" "/app/Lib/File.hs" | ||
-- Just "Lib/File.hs" | ||
stripFilePath :: FilePath -> FilePath -> Maybe FilePath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this behave with windows file namespaces, such as \\?\
, \\.\
and \??\
. See e.g.
- https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#win32-file-namespaces
- https://support.microsoft.com/en-us/topic/70b92942-a643-2f2d-2ac6-aad8acad49fb
- https://superuser.com/questions/1069055/what-is-the-function-of-question-marks-in-file-system-paths-in-windows-registry/1096784#1096784
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My apologies, my implementation is very naive since I have never needed anything like that (I personally don't even know what windows file namespaces are)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure... I've implemented a filepath AST and parser here: #99
That would be more strict wrt special windows filepaths. I haven't decided yet how to use this AST yet... it's pretty annoying to pattern match on it. Using just lexemes on the other hand throws away some information. If you have ideas, let me know.
| isRelative fp = Just fp | ||
| otherwise = Nothing | ||
stripFilePath dir' fp' | ||
| Just relativeFpParts <- splitDir `stripPrefix` splitFp = Just (joinPath relativeFpParts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the splitpath -> joinpath
idiom can sometimes turn an invalid filepath into a valid one, e.g.: #12 (comment)
| Just relativeFpParts <- splitDir `stripPrefix` splitFp = Just (joinPath relativeFpParts) | ||
| otherwise = Nothing | ||
where | ||
dir = normalise dir' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a friend of ViewPatterns
for this, unless there's a reason we can't use that here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the whole implementation is a bit trash atm, so no reason other than trying to be as simple as possible
stripFilePath' (x:xs) (y:ys) | ||
| x `equalFilePath` y = stripFilePath' xs ys | ||
| otherwise = Nothing | ||
stripFilePath' [] ys = Just ys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
> stripFilePath "lol" "lol"
Just ""
I'm not sure whether we should allow this or not (most of the filepath API allows empty paths), but it at least warrants its own doctest line, because for some it might be unexpected.
This also means this function does not guarantee a valid filepath.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point!
BTW, rust has the same:
stripPrefix("lol", "lol") -> Ok("")
Closes #73
Implementation draft to get the discussion started.