term.ui: fix non-ASCII output on Windows consoles - #28072
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a682ec5f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // TODO: Diff the previous frame against this one, and only render things that changed? | ||
| if !ctx.enable_su { | ||
| C.write(1, ctx.print_buf.data, ctx.print_buf.len) | ||
| write_to_stdout(unsafe { tos(ctx.print_buf.data, ctx.print_buf.len) }) |
There was a problem hiding this comment.
For a freshly initialized Context, print_buf is a default []u8, so its data pointer is nil until the first write; tos() explicitly panics on a nil pointer even when the length is zero. Calling the public flush() on an empty first frame, including the analogous synchronized-update path below, therefore terminates the application, whereas the previous zero-length C.write was a no-op. Guard print_buf.len == 0 or use an empty string without calling tos.
Useful? React with 👍 / 👎.
3a682ec to
b9f9cad
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9f9caddfd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| if !ctx.enable_su { | ||
| C.write(1, ctx.print_buf.data, ctx.print_buf.len) | ||
| write_to_stdout(unsafe { tos(ctx.print_buf.data, ctx.print_buf.len) }) |
There was a problem hiding this comment.
-autofree
When a term.ui application is compiled with -autofree, this non-simple temporary string argument is automatically freed after write_to_stdout returns. Because tos only aliases ctx.print_buf.data rather than copying it, that frees the array's backing storage while Context still retains its pointer and capacity; the next frame can write through freed memory, and later cleanup can double-free it. The synchronized-update branch has the same expression, so pass the buffer through a borrowed/manual-free path instead.
Useful? React with 👍 / 👎.
b9f9cad to
9d17d49
Compare
2 participants
This fixes garbled non-ASCII output from
term.uion Windows consoleswhose output code page is not UTF-8 (e.g. 936/GBK, the default on
Chinese systems): for example, the box-drawing glyph
─is emitted asthe UTF-8 bytes
E2 94 80and decoded into unrelated CJK glyphs.flush()writes the print buffer directly viaC.write, i.e. as rawUTF-8 bytes, which is only correct when the console output code page
is UTF-8. Previously the builtin set that code page to UTF-8 on
startup; after that was removed (c47b8d1, fix #15124), term.ui kept
writing raw UTF-8 bytes, which broke on non-UTF-8 consoles. On
Windows, the output now goes through
print(), which converts UTF-8to UTF-16 when writing to a console. Other platforms and redirected
output are unchanged.
Verified on Windows with console code page 936, where a box border
drawn by
term.uirenders as mojibake before the fix and correctlyafter it (see screenshots below).
v fmtis clean andv test vlib/term/uipasses. The console path cannot be exercised inCI (test stdout is a pipe, not a console), so this fix has no
automated test.
Before
After
