Cache stderr ttyconf, colorize std.log, and fix --webui
#25726
Merged
+662
−584
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jacobly0
force-pushed
the
std-log-colors
branch
from
66c7107 to
28b85cf
Compare
`std.Io.tty.Config.detect` may be an expensive check (e.g. involving syscalls), and doing it every time we need to print isn't really necessary; under normal usage, we can compute the value once and cache it for the whole program's execution. Since anyone outputting to stderr may reasonably want this information (in fact they are very likely to), it makes sense to cache it and return it from `lockStderrWriter`. Call sites who do not need it will experience no significant overhead, and can just ignore the TTY config with a `const w, _` destructure.
Also remove the example implementation from the file doc comment; it's better to just link to `defaultLog` as an example, since this avoids writing the example implementation twice and prevents the example from bitrotting.
Using '--webui' without '--time-report' when there are Run steps in the graph was regressed by ziglang#25029.
andrewrk
approved these changes
Oct 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main purpose of this branch is the second commit, which changes the default implementation of
std.logto colorize the output if the detected TTY config allows it. Here's a comparison of some simple output:Of course, you can still always override the implementation through
root.std_options.logFn.For this new
std.log.defaultLogimplementation, we need to know thestd.Io.tty.Configfor stderr, but we'd ideally like to avoid an extra syscall on every single call tostd.log. I have therefore implemented what I think is a good solution to this, which is forstd.debug.lockStderrWriterto not only return anIo.Writer, but also anIo.tty.Config, which is detected once and then cached globally. I believe this is reasonable because code outputting to stderr will often want to use the TTY config to colorize that output, andlockStderrWriteris already coordinating access to stderr with global state, so it seems fine to add a little more global state. I think the diff for the change ended up cleaning some things up slightly.@andrewrk, I'm requesting your review for the
std.debug.lockStderrWriterAPI change explained above.Also let me know if you'd prefer I hold off on the merge to avoid potentialstd.Ioconflicts.The last commit is just a little fix I snuck in for something I regressed in #25029.