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

feat(stdlib): Add user-friendly file system module #1966

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

alex-snezhko
Copy link
Member

Add a file system module that is more high-level and user-friendly than the current sys/file

Closes #211

Copy link
Member

@spotandjake spotandjake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me, Most of my comments were related to graindoc.

Two other things:

  • Currently the sys/ path is used for more low level wasi interactions. I think we should either move these wasi interactions elsewhere or put this module elsewhere.
  • Are we currently able to add some better streaming functions such as File.open, File.readLine and File.close?

stdlib/sys/fs.gr Outdated Show resolved Hide resolved
stdlib/sys/fs.gr Outdated Show resolved Hide resolved
stdlib/sys/fs.gr Outdated Show resolved Hide resolved
stdlib/sys/fs.gr Outdated Show resolved Hide resolved
stdlib/path.gr Outdated Show resolved Hide resolved
stdlib/sys/fs.gr Outdated Show resolved Hide resolved
Copy link
Member

@ospencer ospencer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I love the API design! My major piece of feedback here is that I don't think this module belongs in the sys subdirectory, but in the regular stdlib folder. The sys folder was really just meant for the low-level wasi stuff (and probably should have been named just wasi).

I envision that moving forward to Preview 2, the wasi folder will just have the generated bindings to each of the wasi standards, and we'll have high-level libraries in the regular standard library. We can note in the module doc that use of that library incurs a dependency on a particular wasi world. For now, we can say that use of the module requires WASI Preview 1.

Copy link
Member

@marcusroberts marcusroberts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read all the way through and didn't spot anything, looks like a nice usable interface too.

@spotandjake
Copy link
Member

Overall, I love the API design! My major piece of feedback here is that I don't think this module belongs in the sys subdirectory, but in the regular stdlib folder. The sys folder was really just meant for the low-level wasi stuff (and probably should have been named just wasi).

I envision that moving forward to Preview 2, the wasi folder will just have the generated bindings to each of the wasi standards, and we'll have high-level libraries in the regular standard library. We can note in the module doc that use of that library incurs a dependency on a particular wasi world. For now, we can say that use of the module requires WASI Preview 1.

Thoughts on maybe in 0.6 we rename the sys folder to wasi and keep it in the sys subdirectory? I do kind of like the separation especially when considering grain for web development or embedded development where you would not have file system access.

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

Successfully merging this pull request may close these issues.

Stdlib: user-friendly filesystem API
4 participants