Skip to content

fix(cycles): draw the payment QR code black on white on any terminal theme - #828

Merged
raymondk merged 1 commit into
cycles-buy/03-gatewayfrom
cycles-buy/04-qr-colours
Oct 20, 2026
Merged

raymondk merged 1 commit into
cycles-buy/03-gatewayfrom
cycles-buy/04-qr-colours

Conversation

@raymondk

Copy link
Copy Markdown
Collaborator

The payment QR code was drawn in the terminal's own colours, so a light theme inverted it and a dark theme often gave grey on near-black; many phone cameras read neither.

When stderr has colour support, every row of the drawing is now set to 256-colour palette entry 231 (white) on entry 16 (black), which sit in the fixed colour cube that themes do not remap, and reset at its end. The qrcode crate still does the encoding and the half-block layout; this only wraps its rows. The terminal-size fit check still measures the plain drawing. Without colour support the plain drawing is printed as before, so the worst case is today's output.

This is the small alternative to #824's hand-rolled coloured renderer. Unit test the_qr_code_is_coloured_black_on_white_row_by_row.


Part of restacking #824 into small PRs, one fix each, stacked on main in this order: cycles-buy/01-json-implies-yes → 02-arg-group → 03-gateway → 04-qr-colours → 05-paste-flags → then the wait, its follow-up text and the --json fields. Each PR targets the one below it; after the one below merges, retarget to main.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Twxau844tMTwyHpQJEbmBo

@raymondk
raymondk requested a review from a team as a code owner October 16, 2026 22:44
@lwshang
lwshang requested a balanced review from Copilot October 19, 2026 08:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused change preserves existing layout and fallback behavior, with no unresolved findings.

No open issues 🎉

What changed in this PR

Makes the icp cycles buy payment QR code readable across terminal themes without replacing the existing renderer.

Changes:

  • Applies fixed black and white colours to each row, resetting afterward.
  • Preserves plain-output fallback and terminal-size checks.
  • Adds a regression test and changelog entry.
File Description
crates/​icp-cli/​src/​commands/​cycles/​buy.rs Adds conditional QR colouring and a row-by-row regression test.
CHANGELOG.md Documents the QR readability fix.

Review effort: Balanced

@raymondk
raymondk added this pull request to stack #835 October 20, 2026 15:44
@raymondk
raymondk force-pushed the cycles-buy/04-qr-colours branch 2 times, most recently from d6f64fb to a2fa343 Compare October 20, 2026 15:45
…theme

The code was drawn in the terminal's own colours, so a light theme
inverted it and a dark theme often gave grey on near-black, and many
phone cameras read neither. When stderr has colour support, every row
is now set to 256-colour palette entry 231 (white) on entry 16 (black),
which sit in the fixed colour cube that themes do not remap, and reset
at its end. The terminal-size fit check still measures the plain
drawing. Without colour support the plain drawing is printed as before,
so the worst case is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Twxau844tMTwyHpQJEbmBo
@raymondk
raymondk force-pushed the cycles-buy/04-qr-colours branch from a2fa343 to f9711db Compare October 20, 2026 17:45
@raymondk
raymondk merged commit 2622b02 into main Oct 20, 2026
106 checks passed
@raymondk
raymondk deleted the cycles-buy/04-qr-colours branch October 20, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants