Skip to content

Commit d6f64fb

Browse files
raymondkclaude
andcommitted
fix(cycles): draw the payment QR code black on white on any terminal 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
1 parent 27d1a07 commit d6f64fb

2 files changed

Lines changed: 59 additions & 7 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ air-gapped signing
1111
* fix: `icp cycles buy --json` and `icp cycles buy -q` imply `--yes`: a machine-readable run has nobody to ask, so neither prompts for confirmation on a terminal nor refuses without one, and the quote stays off stderr.
1212
* fix: `icp cycles buy` says, with neither `--amount` nor `--cycles`, that one of the two is needed; and that a negative `--amount` or `--cycles` (`-1`, `-5t`) is negative, in the same words and with exit status 2 for both, where clap read `-5t` as an unknown flag
1313
* fix: `icp cycles buy` says in words what is wrong with a `--gateway` that is not a canister id, a canister that serves no gateway, and one a boundary node cannot route to, where it printed the replica's IC0536 or the HTTP gateway's routing error; CyclePay and Stripe are named only for CyclePay's gateway, any other is "the gateway"
14+
* fix: `icp cycles buy` draws the payment QR code black on white on any terminal theme. It used to take the terminal's own colours, which a light theme inverts and a dark theme often renders grey on near-black, and many phone cameras read neither. A terminal without colour support gets the plain drawing as before.
1415

1516
# v1.7.0
1617

‎crates/icp-cli/src/commands/cycles/buy.rs‎

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use anyhow::{anyhow, bail};
55
use candid::Principal;
66
use clap::{ArgGroup, Args};
77
use dialoguer::Confirm;
8-
use dialoguer::console::Term;
8+
use dialoguer::console::{Term, colors_enabled_stderr};
99
use icp_app::context::{Context, GetAgentError, GetAgentForEnvError, GetAgentForNetworkError};
1010
use icp_app::identity::IdentitySelection;
1111
use icp_app::identity::manifest::IdentityDefaults;
@@ -479,6 +479,8 @@ fn announce_payable(order: &OrderView, quote: &Quote, output: Output, no_open: b
479479
"(Make the terminal {width} columns by {height} rows or larger to see the \
480480
QR code.)"
481481
);
482+
} else if colors_enabled_stderr() {
483+
eprintln!("{}", black_on_white(&qr));
482484
} else {
483485
eprintln!("{qr}");
484486
}
@@ -762,12 +764,12 @@ fn payable_until(order: &OrderView) -> String {
762764
}
763765

764766
/// The URL as a QR code drawn with half-block characters, two modules per
765-
/// row, for scanning off the terminal: light modules are the blocks, in the
766-
/// terminal's own colours, which scans best light on dark. It takes the
767-
/// lowest error correction (a screen does not smudge) and a two-module quiet
768-
/// zone, the least a scanner needs, so that the Payment Link's code fits an
769-
/// 80x24 terminal with the wait's line below it. `None` only if the URL is
770-
/// too long to encode.
767+
/// row, for scanning off the terminal: light modules are the blocks, dark
768+
/// ones the spaces, in the terminal's own colours (`black_on_white` fixes
769+
/// those where it can). It takes the lowest error correction (a screen does
770+
/// not smudge) and a two-module quiet zone, the least a scanner needs, so
771+
/// that the Payment Link's code fits an 80x24 terminal with the wait's line
772+
/// below it. `None` only if the URL is too long to encode.
771773
fn render_qr(url: &str) -> Option<String> {
772774
use qrcode::render::unicode::Dense1x2;
773775
use qrcode::{EcLevel, QrCode};
@@ -802,6 +804,25 @@ fn render_qr(url: &str) -> Option<String> {
802804
)
803805
}
804806

807+
/// The drawn code, black on white whatever the terminal's theme: every row
808+
/// set to white (256-colour palette entry 231) on black (entry 16) and reset
809+
/// at its end. Left in the terminal's own colours, the code is at the mercy
810+
/// of the theme: a light theme inverts it, and a dark theme's background is
811+
/// often a grey rather than black, and many phone cameras read neither.
812+
/// Entries 16 and 231 are in the fixed colour cube, which themes do not
813+
/// remap as they do the first 16 colours, so the result is the same on all
814+
/// of them. For a terminal without colour support the caller prints the
815+
/// plain drawing, which is what every terminal got before: the worst case
816+
/// is no worse than today. The width and height to fit on the screen are
817+
/// those of the plain drawing; measure that, not this.
818+
fn black_on_white(drawing: &str) -> String {
819+
drawing
820+
.lines()
821+
.map(|row| format!("\x1b[38;5;231;48;5;16m{row}\x1b[0m"))
822+
.collect::<Vec<_>>()
823+
.join("\n")
824+
}
825+
805826
/// Where the order stands, for a script: one JSON object, or the status tag
806827
/// alone. A person is told in prose by the caller.
807828
fn print_final(
@@ -1063,6 +1084,36 @@ mod tests {
10631084
assert!(render_qr(&"x".repeat(10_000)).is_none());
10641085
}
10651086

1087+
#[test]
1088+
fn the_qr_code_is_coloured_black_on_white_row_by_row() {
1089+
let url = "https://buy.stripe.com/6oU7sLg7t4ih4LtgwX3VC00?client_reference_id=o_co-1-0000abcd&prefilled_amount=1000";
1090+
let plain = render_qr(url).unwrap();
1091+
let coloured = black_on_white(&plain);
1092+
const SET: &str = "\x1b[38;5;231;48;5;16m";
1093+
const RESET: &str = "\x1b[0m";
1094+
let rows: Vec<&str> = coloured.lines().collect();
1095+
assert_eq!(rows.len(), plain.lines().count());
1096+
// Every row is set and reset on its own, so nothing bleeds into the
1097+
// line after the code, and stripped of the escapes it is the plain
1098+
// drawing again.
1099+
assert!(
1100+
rows.iter()
1101+
.all(|row| row.starts_with(SET) && row.ends_with(RESET)),
1102+
"{coloured:?}"
1103+
);
1104+
let stripped: Vec<String> = rows
1105+
.iter()
1106+
.map(|row| {
1107+
row.strip_prefix(SET)
1108+
.and_then(|row| row.strip_suffix(RESET))
1109+
.unwrap()
1110+
.to_owned()
1111+
})
1112+
.collect();
1113+
assert_eq!(stripped.join("\n"), plain);
1114+
assert!(!stripped.iter().any(|row| row.contains('\x1b')));
1115+
}
1116+
10661117
#[test]
10671118
fn only_a_delivery_ends_well() {
10681119
let order = |status| OrderView {

0 commit comments

Comments
 (0)