Additional OsPath/OsString implementation for Path#198
Open
clintonmead wants to merge 2 commits into
Open
Conversation
This exposes the library interface under the type OsString, which is actually a platform independent interface, instead of exposing WindowsString and PosixString directly in the interface
This is so we can we can produce platform strings when required for functions in this library in the same way for all modules in this library, easing migration between the modules
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OsPath.PosixandOsPath.Windowshave in their APIs references toPosixPathandWindowsPathrespectively.Other libraries tend to use
OsPath, which is just a type alias forOsString, butOsStringis not a type alias forPosixPath/WindowsPath, it's a newtype wrapper. Hence interacting with libraries which useOsPath/OsStringas their underlying type is a bit of a pain.So this PR adds an
OsPath.OsStringmodule, much likeOsPath.PosixandOsPath.Windowswhich already exist.The nice thing about
OsPath.OsStringis that code written against it works on both Posix and Windows, whereas code written againstOsPath.Posixwould need to have it's module imports changed to work on Windows and via versa for code written againstOsPath.Windows. Whilst this is relatively trivial it does mean that one can't make platform independent code with this library currently. But withOsPath.OsStringis actually multiplatform unlike the alternatives and should be able to be run on Windows and Posix without code changes.I haven't really fleshed out test cases but I have got this working on an internal project. If you're happy with the general direction of the PR I'm happy to take the time to add the test cases as you think appropriate.
(Oh I just noticed this basically implements #197)