feat(core): add more IO extensions (exists?, mkdir, rmdir, cwd)#1518
feat(core): add more IO extensions (exists?, mkdir, rmdir, cwd)#1518sqrew wants to merge 4 commits intocarp-lang:masterfrom
Conversation
…set-cwd) Adds cross-platform wrappers for mkdir, rmdir, getcwd, and chdir to IO.Raw and high-level Result-returning versions to the IO module. Also fixes several typos in existing IO documentation and updates tests to cover new functionality.
| #include <direct.h> | ||
| #define getcwd _getcwd | ||
| #define chdir _chdir | ||
| #else |
There was a problem hiding this comment.
It looks like Windows is not happy with this change quite yet.
sqrew
force-pushed
the
stdlib-io-extensions
branch
from
04d80da to
c9fba4a
Compare
|
Added cross-platform Windows implementation for |
| } | ||
| closedir(d); | ||
| } else { | ||
| result.len = -1; |
There was a problem hiding this comment.
I don’t think returning an array of length -1 is safe or sensible here. We should find a different error condition (also for all the functions on top of this one).
| (doc file-exists? "Checks if a file exists at the given path.") | ||
| (register file-exists? (Fn [&String] Bool)) | ||
|
|
||
| (doc dir-exists? "Checks if a directory exists at the given path.") | ||
| (register dir-exists? (Fn [&String] Bool)) | ||
|
|
There was a problem hiding this comment.
Those should probably also be in IO.Raw.
| (let [res (IO.Raw.get-cwd)] | ||
| (if (null? res) | ||
| (Result.Error (fmt "Failed to get current working directory: %s" &(System.error-text))) | ||
| (let [s (String.from-cstr res)] |
There was a problem hiding this comment.
The C side already returns a String. We should fix the function signature on the C side to avoid a double allocation.
| "" | ||
| "Returns a (Result Bool String) indicating success or failure.") | ||
| (defn copy-file [src dest] | ||
| (Result.and-then (read-file src) |
There was a problem hiding this comment.
This reads the complete file into memory. We should probably do a chunked read-write.
| #ifdef _WIN32 | ||
| return _mkdir(*path); | ||
| #else | ||
| return mkdir(*path, 0777); |
There was a problem hiding this comment.
Is using 0777 really desirable here?
| (doc open-file | ||
| "Opens a [FILE](#file) with the given name using a designated mode" | ||
| "(e.g. [r]ead, [w]rite, [a]ppend), [rb] read binary...)." | ||
| "(e.g. [r]ead, [w]rite, [a]ppend), [rb] read binary...." |
There was a problem hiding this comment.
We open a paren here, so we should also close it.
| data | ||
| &(write-then-read data "io_carp_testdata.txt") | ||
| "write-file then read-file" )) | ||
| (let [res (write-then-read data "io_carp_testdata.txt")] |
There was a problem hiding this comment.
Why the let change here? Should be fine without?
| "append-file appends to existing file")))) | ||
| (do | ||
| (IO.Raw.unlink! file-name) | ||
| (let [s (Result.unsafe-from-success result)] |
There was a problem hiding this comment.
Same let-change here. Why? We also don’t need a do inside a let-do.
- Fixed IO.Raw.get-cwd to avoid double allocation. - Refactored IO.Raw.list-dir to use out-parameter instead of -1 length. - Improved IO.copy-file with chunked read-write loop. - Tightened IO.Raw.mkdir permissions to 0755. - Cleaned up stylistic issues (redundant do/let blocks). - Fixed typos and syntax.
| "" | ||
| "Returns a (Result String String) indicating success or failure.") | ||
| (defn get-cwd [] | ||
| (let [res (IO.Raw.get-cwd)] |
There was a problem hiding this comment.
Ths can return NULL. We need to check for that.
| "Returns a (Result Bool String) indicating success or failure.") | ||
| (defn copy-file [src dest] | ||
| (let [f-src? (open-file src "rb") | ||
| f-dest? (open-file dest "wb")] |
There was a problem hiding this comment.
Ths will create and open the file before we validated the source is valid.
| (let-do [f-src (Result.unsafe-from-success f-src?) | ||
| f-dest (Result.unsafe-from-success f-dest?) | ||
| buf-size 4096 | ||
| buffer (String.allocate buf-size \0) |
There was a problem hiding this comment.
Can we validate the allocation like we do in read-file?
| HANDLE hFind = FindFirstFile(search_path, &fd); | ||
| if (hFind != INVALID_HANDLE_VALUE) { | ||
| out_result->len = 0; | ||
| out_result->capacity = 0; |
| } | ||
| String name = String_from_MINUS_cstr(dir->d_name); | ||
| out_result->len++; | ||
| out_result->data = CARP_REALLOC(out_result->data, sizeof(String) * out_result->len); |
There was a problem hiding this comment.
Don’t reallocate on every directory, this is quadratic. Grow geometrically like arrays do.
|
IO is more complicated than I anticipated so I will close this in favor of someone more qualified attempting it |
2 participants
Adds several cross-platform IO extensions to the standard library:
IO.file-exists?andIO.dir-exists?IO.create-dir(wraps mkdir)IO.remove-dir(wraps rmdir)IO.get-cwd(wraps getcwd)IO.set-cwd(wraps chdir)Includes corresponding cross-platform C wrappers in
core/carp_io.hand comprehensive tests intest/io.carp.