feat: add --character-table-file for custom JSON character tables (fixes #262)#277
feat: add --character-table-file for custom JSON character tables (fixes #262)#277leno23 wants to merge 2 commits into
Conversation
Load a JSON array of 256 single-character strings from a file or via HEXYL_CHARACTER_TABLE_FILE. Fixes sharkdp#262. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f7f2484d4
ℹ️ 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".
| long, | ||
| value_name("FILE"), | ||
| env = "HEXYL_CHARACTER_TABLE_FILE", | ||
| conflicts_with("character_table") |
There was a problem hiding this comment.
Allow CLI preset to override env-provided table file
Using conflicts_with("character_table") on character_table_file makes the env fallback behave like a hard conflict: when HEXYL_CHARACTER_TABLE_FILE is set and a user passes --character-table=..., clap treats both as present at runtime and exits with an argument conflict instead of letting the CLI option win. This breaks common CLI precedence (explicit flag over environment) and makes one-off overrides impossible in environments where the variable is exported.
Useful? React with 👍 / 👎.
| #[derive(Clone, Debug)] | ||
| pub enum CharacterTableConfig { | ||
| Preset(CharacterTable), | ||
| File(Vec<String>), |
There was a problem hiding this comment.
Enforce invariants for custom character table entries
Exposing CharacterTableConfig::File(Vec<String>) as a public variant allows callers to pass vectors that are not exactly 256 entries (or otherwise invalid), but later rendering indexes this table by byte value and assumes all 256 indices exist. That creates a new panic path for library consumers that did not exist with the previous CharacterTable enum-only API. Consider making the variant private or adding a validated constructor to preserve the 256-entry invariant.
Useful? React with 👍 / 👎.
Summary
--character-table-file FILEto load a user-provided character table from JSON.HEXYL_CHARACTER_TABLE_FILEas an environment variable fallback.--character-tablepreset selection.Fixes #262.
Test plan
cargo test character_tabletests/examples/custom-char-table.json, invalid table rejection, and clap conflict with--character-table