fix(unit-only): Add one glyph abstraction and route all TUI/console glyph... (#29)#29
Draft
aidandaly24 wants to merge 1 commit into
Draft
fix(unit-only): Add one glyph abstraction and route all TUI/console glyph... (#29)#29aidandaly24 wants to merge 1 commit into
aidandaly24 wants to merge 1 commit into
Conversation
…ack for legacy Windows consoles
Route all TUI selection cursors, checkboxes, scroll arrows, step-indicator
marks, and deploy progress glyphs through a single src/cli/tui/utils/symbols.ts
helper that detects Unicode support (honoring an AGENTCORE_ASCII override) and
falls back to ASCII ('>', 'x', '^'/'v', '*'/'o', '->', '[x]'/'[ ]') when the
terminal cannot render BMP glyphs. Prevents mojibake on legacy Windows CMD /
non-UTF-8 PowerShell while preserving the original glyphs on modern terminals.
No new runtime dependency.
Fixes #29
Coverage Report
|
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.
Refs aws#29
Issues
Root cause
The CLI hardcodes BMP Unicode glyphs and emoji as raw string literals across its Ink component tree and console output with no Unicode-support detection and no ASCII fallback. Personally verified at v0.20.2: SelectList.tsx:57 ('❯'),48('↑'),67('↓'); MultiSelectList.tsx:42('↑'),47('[✓]'),51('❯'),59('↓'); AwsTargetConfigUI.tsx:175('❯'),176('[✓]'); StepIndicator.tsx:74('✓'/'●'/'○'),84('→'); AddGatewayScreen.tsx:286 and AddPaymentManagerScreen.tsx:221 ('[✓]'); HomeScreen.tsx:31 ('⚑'); commands/deploy/progress.ts:37,39 (console.log '✓'/'✗'). '❯' confirmed as UTF-8 e2 9d af (U+276F). No figures/is-unicode-supported dep; ink v6 Text.js has zero unicode handling, so no upstream fallback exists.
The fix
Add one glyph abstraction and route all TUI/console glyphs through it. Option A (lower risk): add
figures(maps every glyph used here to ASCII via is-unicode-supported) and swap the literals at the cited file:lines. Option B: hand-roll src/cli/tui/utils/symbols.ts that detects Unicode support and exports cursor/check/arrow/dot constants with ASCII fallbacks ('>','x','^'/'v','*'/'o'). Apply the same to emoji sites and deploy/progress.ts. Optionally force chcp 65001 on Windows at startup for the mis-decode case. One layer fixes both #29 and #28. Design decision: figures dependency vs hand-rolled table.Files touched: src/cli/tui/components/SelectList.tsx:48,57,67; MultiSelectList.tsx:42,47,51,59; AwsTargetConfigUI.tsx:175,176; StepIndicator.tsx:74,84; src/cli/tui/screens/mcp/AddGatewayScreen.tsx:286; src/cli/tui/screens/payment/AddPaymentManagerScreen.tsx:221; src/cli/tui/screens/home/HomeScreen.tsx:31; src/cli/commands/deploy/progress.ts:37,39; plus ~40 other emoji sites under src/cli/. New shared helper src/cli/tui/utils/symbols.ts (or add
figuresto package.json). Optional UTF-8 codepage setup in src/cli/index.ts / src/cli/cli.ts.Validation evidence
The fix was verified by reproducing the original symptom and re-running after the change:
AFTER (fix/29, build EXIT 0 -> OK .../dist/cli/index.mjs): a single symbols.ts abstraction with isUnicodeSupported() (honors AGENTCORE_ASCII override + is-unicode-supported-style heuristic, no new dependency) drives all 8 named call sites. Rendered-text evidence captured under both modes:
Provided symbols.test.tsx passes 6/6; my independent adversarial repro test confirms the fix.
Test suite: green.
Staged on the fork as a draft for human review. Promote to aws/agentcore-cli after vetting.