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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove MemoryFS dependency on Windows Path implementation #1241

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gderham
Copy link

@gderham gderham commented Aug 19, 2023

Hi, long term fan of language-ext 馃槃

I ran all LanguageExt.Tests tests on macOS (.NET 7.0) and some fail, mostly due to MemoryFS using Path.IsPathRooted(), Path.DirectorySeparatorChar etc with error:

System.IO.IOException : Path not rooted: C:
   at LanguageExt.Sys.MemoryFS.ParsePath(String path) in /Users/_/code/language-ext/LanguageExt.Sys/MemoryFS.cs:line 51

In this PR I've modified MemoryFS and MemoryFSTests to behave the same independent of the host OS, and therefore enable the tests to pass. The replacement methods aren't 100% realistic but I hope acceptable for this usage. There remains a dependency on System.IO for some exceptions and other types.

MemoryFS use of Windows file paths is unchanged so there is no breaking change.

Thanks

@gderham gderham marked this pull request as ready for review August 19, 2023 17:21
@louthy
Copy link
Owner

louthy commented Aug 19, 2023

Thanks for this, I'll have a look in more detail next week. My initial thoughts on a quick scan through is that probably LanguageExt.Sys needs its own Path trait like the other traits (with platform specific implementations). We can then do Path<RT>.* to get to the injected path-functionality.

I had naively thought that Path wouldn't have side-effects, I should never have given MS the benefit of that doubt!

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

Successfully merging this pull request may close these issues.

None yet

2 participants