Pass io to zeit.instant only when needed #59
Conversation
Add new EnvConfig struct to replace ?*const std.process.EnvMap parameter. This struct has two optional fields: tz and tzdir, which are the only env vars used in the codebase (TZ and TZDIR). Amp-Thread-ID: https://ampcode.com/threads/T-019c1943-dfe8-738f-bb29-33f5535caba6 Co-authored-by: Amp <amp@ampcode.com>
Change local(alloc, maybe_env) to local(alloc, io, env) to use the new
EnvConfig struct and Zig 0.16's io parameter for file readers.
- Replace ?*const std.process.EnvMap with EnvConfig parameter
- Use env.tz instead of env.get("TZ")
- Pass io to f.reader() as required by Zig 0.16
- Update localFromEnv call to pass io parameter
Amp-Thread-ID: https://ampcode.com/threads/T-019c1944-707c-729d-bd53-54f720ca9a4b
Co-authored-by: Amp <amp@ampcode.com>
Update localFromEnv() to accept std.Io and EnvConfig parameters.
Update loadTimeZone() to accept std.Io and EnvConfig, replacing the
optional EnvMap parameter. Change env.get("TZDIR") to env.tzdir.
Fix f.reader() calls to use the new two-argument API.
Amp-Thread-ID: https://ampcode.com/threads/T-019c1944-ff40-7226-a919-97166bf2902d
Co-authored-by: Amp <amp@ampcode.com>
Add io parameter to instant() function signature and replace the removed std.time.nanoTimestamp() with std.Io.Clock.now(.real, io). Update all callers in tests to pass std.testing.io. Amp-Thread-ID: https://ampcode.com/threads/T-019c1945-d480-712c-b7cf-944f207f6048 Co-authored-by: Amp <amp@ampcode.com>
Update test code to use Zig 0.16's new Writer API: - Replace std.io.fixedBufferStream(&buf) with std.Io.Writer.fixed(&buf) - Replace fbs.writer() calls with direct &writer references - Replace fbs.reset() with writer.end = 0 - Replace fbs.getWritten() with writer.buffered() Affects fmtStrftime and gofmt tests. Amp-Thread-ID: https://ampcode.com/threads/T-019c1948-1e3c-71c0-acbc-362fd1c33a22 Co-authored-by: Amp <amp@ampcode.com>
- Replace std.io.Reader with std.Io.Reader in TZInfo.parse - Replace writeByteNTimes with splatByteAll - Update local() call in Instant test to use new 3-arg signature - Replace std.io.null_writer with std.Io.Writer.Discarding - Replace ArrayList.writer() with std.Io.Writer.Allocating in tests - Update loadTimeZone calls to pass std.testing.io and EnvConfig Amp-Thread-ID: https://ampcode.com/threads/T-019c194a-2af5-75d1-8b83-8635b95d7d7e Co-authored-by: Amp <amp@ampcode.com>
Replace deprecated std.fs file and directory operations with their std.Io equivalents for Zig 0.16 compatibility: - std.fs.cwd().openFile -> std.Io.Dir.cwd().openFile(io, ...) - std.fs.openDirAbsolute -> std.Io.Dir.cwd().openDir(io, ...) - std.fs.Dir -> std.Io.Dir - f.close() -> f.close(io) - dir.close() -> dir.close(io) - dir.openFile(...) -> dir.openFile(io, ...) Amp-Thread-ID: https://ampcode.com/threads/T-019c194f-494e-7058-9236-403b366f06ad Co-authored-by: Amp <amp@ampcode.com>
…formatting `timeFmt` supports both `gofmt` and `strftime`
…an `now` requires it
|
Is there a pattern emerging here? I feel like passing it in on the config is not how this would be done with an allocator. What do you think about making it a nullable param?
|
I think it might be nicer to have two functions, one with a config, and one without but it takes an fn instant(config: Config)
fn instantNow(io: std.Io, tz: TimeZone)The new |
|
I agree that separating creating |
|
Is this PR stuck or just "asleep"? I see it's stale, the target branch has been force-pushed, and at this point master might be the better target. Is there something I could do to help it get merged? I would prefer the 2-function solution as suggested by @eamonburns fn instant(config: Config)
fn instantNow(io: std.Io, tz: TimeZone) |
|
or maybe another way: split the falible text-parsing functions from the rest, and for const TimeSource = union(enum) {
now: std.Io,
unix_timestamps: Seconds,
// ...
}
fn instant(time_source: TimeSource, tz: *const TimeZone) Instant;
const TextFormat = enum {
iso8601,
rfc3399,
// ..
}
fn instantFromText(text_format: TextFormat, text: []const u8, tz: *const TimeZone) ParseError!Instant;Since text may contain TZ info, the meaning of |
|
Oh that seems pretty nice. I think now is the only instance you would ever need an Io. |
|
I've started experimenting a bit more in my own branch so feel free to look it's not ready for PR yet, though. I'm not even sure I like the sput API better yet, I want to use it in my projects a bit first. So far some of my tests fail (i skip them) because apparently the iso parsing does not respect the (Btw I'm |
|
Ah nice, please PR if the api feels good! See you on irc |
|
So I've prepared alternative PR #69, splitting the API to two functions -- one fallible and one infallible -- more along with what @eamonburns has suggested. |
|
Superseded by #69. |
No source other than
nowrequires it