regz: Move VirtualFilesystem into /modules#983
Conversation
Grazfather
left a comment
Grazfather
left a comment
There was a problem hiding this comment.
Assuming that root.zig is basically an unchanged VirtualFilesystem.zig, it looks good to me.
Why virtual-io instead virtual fs?
mattnite
left a comment
mattnite
left a comment
There was a problem hiding this comment.
I don't see us needing this to become a full virtual IO system, it's just an in-memory filesystem for easy testing, so I'd keep the VirtualFilesystem name for both the module directory name and the file name.
| const builtin = @import("builtin"); | ||
|
|
||
| const Allocator = std.mem.Allocator; | ||
| const assert = std.debug.assert; |
There was a problem hiding this comment.
This is unused according to our check
There was a problem hiding this comment.
Only assert is unused, fixed
|
I figured the name was picked before the introduction of the Io interface, so if it was created now it would be called VirtualIo. But I don't have a strong opinion on this, I'll just revert this change. |
|
I do not mind the naming, I was only curious. I will actually review the changes now. Good work on the bug fixes! |
| pub const kind: Node.Kind = empty; | ||
|
|
||
| pub const ID = enum(Node.ID) { | ||
| // Reserve first 256 file handles for os-specific values, such as sitdin/out/err |
There was a problem hiding this comment.
| // Reserve first 256 file handles for os-specific values, such as sitdin/out/err | |
| // Reserve first 256 file handles for os-specific values, such as stdin/out/err |
| } | ||
| }; | ||
|
|
||
| pub fn from_handle(T: type, handle: std.posix.fd_t) !T { |
There was a problem hiding this comment.
Shouldn't these return std.Io.Operation.FileWriteStreaming.Error? Since we try in write_streaming which returns that error type.
There was a problem hiding this comment.
from_handle is used in a few places and FileWriteStreaming.Error set just does not contain the FileNotFound error. I guess the closest would be NotOpenForWriting?
I didn't spend too much time picking these error names, in case they are encountered the trace should point to the exact error location anyway.
| } | ||
|
|
||
| pub fn to_handle(id: anytype) std.posix.fd_t { | ||
| // const ID = @TypeOf(id); |
|
Was this merge intentional? There were still issues to be resolved 😅 |
3 participants
I originally intended for this to just be a simple PR moving the files but the VirtualFilesystem code was offensive enough that I decided to rewrite most of it, adding more error checking along the way.
I also found and fixed a bug where Dir.openFile() only matched on the file name, but not on directory names, so if both
foo/bar/main.zigand
alice/bob/main.zigexisted, handle to any of them could be returned.