feat(encoding): name and decode text encodings - #665
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c462066d7c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
andiwand
force-pushed
the
feat/text-encoding
branch
from
August 9, 2026 08:17
c462066 to
ede0212
Compare
`TextFile::charset()` was a stub returning nothing, and the charset uchardet guessed was used nowhere — every non-UTF-8 text file rendered as mojibake because `html/text_file.cpp` piped raw bytes into a document declaring UTF-8. Detection also drained the whole stream, so classifying one unrecognised large file cost a full pass over it. A new `internal/encoding` package carries a public `TextEncoding` enum with a name table behind it, mirroring `file_type_table`: canonical name first, aliases after, matching that ignores case and `-`/`_`/space so `windows-1252`, `WINDOWS_1252` and `cp1252` land together. uchardet returns a name; this is what maps it home. Decoding covers UTF-8/16/32 and the WHATWG single-byte set, generated from Python's codecs by `tools/encoding/generate_encoding_data.py`. Malformed input becomes U+FFFD rather than an error: a decoder that throws half way through leaves the caller with nothing, and a wrong guess is the expected failure. The multi-byte legacy encodings are named but not decoded, which is enough to render them as text with their own charset in the html header and let the browser do the work. Detection now reads a bounded probe. Nothing rejects: text is the fallback for bytes nothing else claims, so a viewer handed a random binary shows junk rather than an error, and bytes we cannot name are `TextEncoding::unknown` rather than a throw. `TextFile::text()` now returns what its name promises — the file's text, decoded to UTF-8 — where before it returned raw bytes and a Latin-1 file came back as mojibake. `stream()` is unchanged for callers that want the bytes. `TextFile::charset()` stays, deprecated and now actually working, in terms of the new `TextFile::encoding()`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSgWdLTSLCWDeFvbVwZDVU
andiwand
force-pushed
the
feat/text-encoding
branch
from
August 9, 2026 08:33
ede0212 to
cbc058a
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🤖 Generated with Claude Code
First of five stacked PRs driving the CSV implementation toward a spreadsheet interface. The plan lives in
src/odr/internal/csv/PLAN.md, added here.No csv in this one. It builds the substrate the rest stands on, and closes a live bug on its own.
What was wrong
TextFile::charset()was a stub returning{}.html/text_file.cpppiped raw bytes into a document declaring<meta charset="UTF-8">(// TODO charset), so every non-UTF-8 text file rendered as mojibake.guess_charsetdrained the whole stream, so classifying one unrecognised large file cost a full pass over it — twice over, since the csv probe did the same.What this adds
internal/encoding, a package of its own rather than a corner ofinternal/text, because it is whatpdfwould later share.TextEncodingenum with a name table mirroringfile_type_table: canonical name first, aliases after, one row per encoding, and a test that fails when an alias is claimed twice. Matching ignores case and-/_/space, sowindows-1252,WINDOWS_1252andcp1252land on the same row. uchardet hands back a name; this is what maps it home.to_utf8covering UTF-8/16/32 and the WHATWG single-byte set. Tables generated from Python's own codecs bytools/encoding/generate_encoding_data.py, in the output shapetools/pdfalready uses.open_strategy.cpp:276leans on, sotext::TextFilenow throwsNoTextFilewhere it threwUnknownCharset.Malformed input becomes U+FFFD rather than an error: a decoder that throws half way through a document leaves the caller with nothing, and a wrong guess is the expected failure here — that is what the override in the next PRs is for.
Multi-byte legacy encodings (Shift-JIS, GBK, Big5, EUC-KR, …) are named but not decoded. That is enough to render them as text with their own charset in the html header and let the browser decode. Only the spreadsheet path, which must hand UTF-8 to the bindings, is closed to them.
PLAN.mdrecords the two routes for closing that gap and their costs.API
TextFile::encoding()— new, and the first working answer.TextFile::charset()— kept,[[deprecated]], implemented viaencoding(). Bindings moved off it so the attribute stays quiet.TextFile::text()— now decodes to UTF-8 where it can, raw bytes where it cannot.UnknownCharset— deprecated; nothing throws it any more.Tests
22 new, covering the table's uniqueness and round-tripping, single-byte/UTF-16/UTF-32 decoding, U+FFFD on broken input, BOM handling (a mark is only stripped by its own encoding), and the binary gate. Full suite green, including all 234 reference-output tests — no output drift.