Show QR code image on terminal along with text - #526
Conversation
📝 WalkthroughWalkthroughA new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main.rs (1)
98-109:⚠️ Potential issue | 🟠 MajorRendering QR for
Event::PairingCodeseems incorrect.
Event::PairingCodeprovides a short code (like "12345678") that users manually type into WhatsApp via "Link with phone number instead". This code is not meant to be scanned — the user is already on their phone and can't scan a QR code from the same device.Rendering a QR code for this event is semantically incorrect and may confuse users. Consider removing the
print_qr_to_terminalcall for this event type.Proposed fix
Event::PairingCode { code, timeout } => { info!("========================================"); info!("PAIR CODE (valid for {} seconds):", timeout.as_secs()); info!("Enter this code on your phone:"); info!("WhatsApp > Linked Devices > Link a Device"); info!("> Link with phone number instead"); info!(""); info!(" >>> {} <<<", code); - print_qr_to_terminal(&code); info!(""); info!("========================================"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 98 - 109, The Event::PairingCode branch is rendering a QR which is incorrect for the short manual pairing code; remove the call to print_qr_to_terminal(&code) from the Event::PairingCode arm so only the textual instructions and the code (variable code) are shown, leaving the rest of the info! logs intact (the match arm handling Event::PairingCode in src/main.rs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main.rs`:
- Around line 33-36: The custom_code branch currently prints and renders a QR
which is inappropriate for an 8-char manual pair code; update the block that
checks custom_code so it only prints the code (remove the call to
print_qr_to_terminal) and avoid introducing logging calls before
env_logger::init(); if you need to log here instead of eprintln!, move the
env_logger::init() call earlier (or defer logging until after init). Ensure you
reference the custom_code variable and the print_qr_to_terminal function (and
consider consistency with Event::PairingCode handling) when making the change.
- Around line 267-275: Replace the panic-prone .unwrap() in print_qr_to_terminal
by handling the Result from QrCode::new(txt): return a Result from
print_qr_to_terminal or log and early-return on Err so the app doesn't crash
when input exceeds QR capacity; locate the QrCode::new(txt) call and propagate
or map the error into a useful message. Also replace the redundant "what"
comments around code.render::<unicode::Dense1x2>().build() and printing with a
short "why" comment explaining why Dense1x2 rendering was chosen (e.g., better
aspect ratio for terminal display) or remove them entirely. Ensure callers are
updated if you change the function signature to return Result.
---
Outside diff comments:
In `@src/main.rs`:
- Around line 98-109: The Event::PairingCode branch is rendering a QR which is
incorrect for the short manual pairing code; remove the call to
print_qr_to_terminal(&code) from the Event::PairingCode arm so only the textual
instructions and the code (variable code) are shown, leaving the rest of the
info! logs intact (the match arm handling Event::PairingCode in src/main.rs).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 35560737-ee2d-4489-b339-e53396728d5f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
Cargo.tomlsrc/main.rs
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/main.rs (2)
33-36:⚠️ Potential issue | 🟡 MinorAvoid rendering manual pair codes as QR.
Line 35 and Line 106 render manual/code-entry pairing values as QR.
Event::PairingCodeis a phone-entry flow, so QR output is misleading noise.Suggested diff
if let Some(ref code) = custom_code { eprintln!("Custom pair code: {}", code); - print_qr_to_terminal(&code); } @@ Event::PairingCode { code, timeout } => { @@ info!(" >>> {} <<<", code); - print_qr_to_terminal(&code); info!("");Also applies to: 98-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 33 - 36, The code is rendering manual pairing codes as QR which is misleading; remove or guard calls to print_qr_to_terminal so that manual/code-entry flows (e.g., when custom_code is Some or when handling Event::PairingCode) do not emit a QR; instead only print the textual code (eprintln) for those paths and call print_qr_to_terminal only for the QR pairing flow. Locate the usages of custom_code and the handling of Event::PairingCode and update them to skip or conditionally call print_qr_to_terminal accordingly.
266-275:⚠️ Potential issue | 🟡 MinorHandle QR code generation errors explicitly and improve comment clarity.
Line 268 silently drops
QrCode::new(txt)failures with no signal, making missing QR output hard to diagnose. Additionally, the comments on lines 269 and 272 describe what the code does ("Render QR code", "Print to terminal") rather than why (e.g., why Dense1x2 encoding is chosen for terminal spacing).Suggested diff
pub fn print_qr_to_terminal(txt: &str) { - if let Ok(code) = QrCode::new(txt) { - // Render QR code as UTF-8 characters for the terminal - let qr_string = code.render::<unicode::Dense1x2>().build(); - - // Print to terminal - println!("{}", qr_string); - } + match QrCode::new(txt) { + Ok(code) => { + // Dense1x2 keeps terminal QR modules visually close to square. + let qr_string = code.render::<unicode::Dense1x2>().build(); + println!("{}", qr_string); + } + Err(err) => eprintln!("Failed to render QR in terminal: {err}"), + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 266 - 275, The function print_qr_to_terminal currently ignores failures from QrCode::new(txt); update print_qr_to_terminal to handle errors explicitly (either return Result<(), Error> or log/print a clear error message) when QrCode::new fails and do not silently drop the failure; also update the inline comments around render::<unicode::Dense1x2>().build() to explain why the Dense1x2 encoding is chosen (it preserves aspect ratio/spacing for terminal characters) rather than describing only what the call does, and keep references to QrCode::new, code.render::<unicode::Dense1x2>().build(), and println! so reviewers can locate the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main.rs`:
- Around line 33-36: The code is rendering manual pairing codes as QR which is
misleading; remove or guard calls to print_qr_to_terminal so that
manual/code-entry flows (e.g., when custom_code is Some or when handling
Event::PairingCode) do not emit a QR; instead only print the textual code
(eprintln) for those paths and call print_qr_to_terminal only for the QR pairing
flow. Locate the usages of custom_code and the handling of Event::PairingCode
and update them to skip or conditionally call print_qr_to_terminal accordingly.
- Around line 266-275: The function print_qr_to_terminal currently ignores
failures from QrCode::new(txt); update print_qr_to_terminal to handle errors
explicitly (either return Result<(), Error> or log/print a clear error message)
when QrCode::new fails and do not silently drop the failure; also update the
inline comments around render::<unicode::Dense1x2>().build() to explain why the
Dense1x2 encoding is chosen (it preserves aspect ratio/spacing for terminal
characters) rather than describing only what the call does, and keep references
to QrCode::new, code.render::<unicode::Dense1x2>().build(), and println! so
reviewers can locate the changes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main.rs (1)
97-106:⚠️ Potential issue | 🟠 MajorDo not render
Event::PairingCodeas a QR image.At Line 105,
Event::PairingCodeis being rendered as QR, but this event carries the 8-character manual linking code (phone entry flow), not QR scan payload. This adds noisy output and may mislead users.Suggested change
Event::PairingCode { code, timeout } => { info!("========================================"); info!("PAIR CODE (valid for {} seconds):", timeout.as_secs()); info!("Enter this code on your phone:"); info!("WhatsApp > Linked Devices > Link a Device"); info!("> Link with phone number instead"); info!(""); info!(" >>> {} <<<", code); - print_qr_to_terminal(&code); info!(""); info!("========================================"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 97 - 106, Event::PairingCode is being treated as a QR payload; remove the QR rendering for this case by eliminating the print_qr_to_terminal(&code) call (or guard it) inside the Event::PairingCode arm so only the textual 8-character manual linking code is displayed via info! macros. Locate the Event::PairingCode match arm and ensure print_qr_to_terminal is not invoked for the variable code (and keep the existing info! lines that show timeout and the code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main.rs`:
- Around line 94-95: The sync function print_qr_to_terminal currently does
CPU-heavy QR rendering and blocking println! calls (and ignores QrCode::new
errors) while being called from async handlers; change print_qr_to_terminal to
async and move its blocking work into tokio::task::spawn_blocking so the async
runtime isn’t blocked, propagate or log QrCode::new errors instead of ignoring
them, and await the spawned task from callers at the existing call sites where
print_qr_to_terminal(&code) is invoked.
---
Duplicate comments:
In `@src/main.rs`:
- Around line 97-106: Event::PairingCode is being treated as a QR payload;
remove the QR rendering for this case by eliminating the
print_qr_to_terminal(&code) call (or guard it) inside the Event::PairingCode arm
so only the textual 8-character manual linking code is displayed via info!
macros. Locate the Event::PairingCode match arm and ensure print_qr_to_terminal
is not invoked for the variable code (and keep the existing info! lines that
show timeout and the code).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| print_qr_to_terminal(&code); | ||
| info!("----------------------------------------"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file size to determine how to read it
wc -l src/main.rsRepository: jlucaso1/whatsapp-rust
Length of output: 81
🏁 Script executed:
# Read the relevant sections - around lines 94-95 (callsite)
sed -n '85,110p' src/main.rsRepository: jlucaso1/whatsapp-rust
Length of output: 1504
🏁 Script executed:
# Read the function definition around lines 267-272
sed -n '260,280p' src/main.rsRepository: jlucaso1/whatsapp-rust
Length of output: 422
🏁 Script executed:
# Also check for any other references to print_qr_to_terminal
rg 'print_qr_to_terminal' -n src/main.rsRepository: jlucaso1/whatsapp-rust
Length of output: 223
🏁 Script executed:
# Check if log/logging is available in the file
head -30 src/main.rs | grep -E "use log|use tracing"Repository: jlucaso1/whatsapp-rust
Length of output: 89
🏁 Script executed:
# Check imports and see what logging is used in the file
rg "log::|info!|warn!" src/main.rs | head -10Repository: jlucaso1/whatsapp-rust
Length of output: 679
Offload QR render/print from async event handling and surface QR generation failures.
print_qr_to_terminal runs CPU-intensive QR code rendering and blocking println! synchronously, but it's called from async event handlers (lines 94 and 105). This blocks Tokio worker threads. Additionally, QrCode::new errors are silently ignored.
Wrap the function body in tokio::task::spawn_blocking and make the function async, as per the coding guideline: "All I/O uses Tokio; wrap blocking I/O and heavy CPU work in tokio::task::spawn_blocking".
Suggested change
- print_qr_to_terminal(&code);
+ print_qr_to_terminal(&code).await;
@@
- print_qr_to_terminal(&code);
+ print_qr_to_terminal(&code).await;
@@
-pub fn print_qr_to_terminal(txt: &str) {
- if let Ok(code) = QrCode::new(txt) {
- // Render QR code as UTF-8 characters for the terminal
- let qr_string = code.render::<unicode::Dense1x2>().build();
-
- // Print to terminal
- println!("{}", qr_string);
- }
+pub async fn print_qr_to_terminal(txt: &str) {
+ let payload = txt.to_owned();
+ let _ = tokio::task::spawn_blocking(move || {
+ match QrCode::new(&payload) {
+ Ok(code) => {
+ let qr_string = code.render::<unicode::Dense1x2>().build();
+ println!("{}", qr_string);
+ }
+ Err(e) => {
+ log::warn!("Failed to generate QR code for terminal output: {}", e);
+ }
+ }
+ })
+ .await;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main.rs` around lines 94 - 95, The sync function print_qr_to_terminal
currently does CPU-heavy QR rendering and blocking println! calls (and ignores
QrCode::new errors) while being called from async handlers; change
print_qr_to_terminal to async and move its blocking work into
tokio::task::spawn_blocking so the async runtime isn’t blocked, propagate or log
QrCode::new errors instead of ignoring them, and await the spawned task from
callers at the existing call sites where print_qr_to_terminal(&code) is invoked.
There was a problem hiding this comment.
Not sure, if it should be implemented. Need to ask the maintainers.
There was a problem hiding this comment.
@abhi3700, that's a reasonable point to raise. Here's a summary of both sides to help frame the discussion with maintainers:
Why the fix matters (the concern is valid):
print_qr_to_terminalperforms CPU work (QrCode::new+ rendering) and a blockingprintln!, all inside anasyncevent handler closure. Per the project's own coding guideline, blocking work in async contexts should be offloaded viatokio::task::spawn_blocking.- If the Tokio runtime is configured with a small thread pool (or is under load), blocking a worker thread—even briefly—can delay other async tasks.
Why the impact is low in practice:
- This is a one-shot operation: the QR code is displayed once during authentication and never again. It's not a hot path.
- QR rendering for a small payload is typically sub-millisecond, so the actual blockage is negligible for a dev/CLI tool.
- The PR description image confirms this is a developer-facing CLI workflow, not a production service.
Recommendation for maintainers:
The fix aligns with the project's stated guideline and is good async hygiene. Whether to require it depends on how strictly they want to enforce that guideline for non-critical code paths. A middle ground would be to at least log the QrCode::new error (currently silently swallowed) even if spawn_blocking is deferred — that part has no downside.
Feel free to share this context with the maintainers when asking for their preference!
🧠 Learnings used
Learnt from: CR
Repo: jlucaso1/whatsapp-rust PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T17:48:51.780Z
Learning: Applies to **/*.rs : All I/O uses Tokio; wrap blocking I/O (`ureq`) and heavy CPU work in `tokio::task::spawn_blocking`
|
Hello @abhi3700. Thanks for your contribution. I'm closing the PR right now for the same reason as this other one. #80 (comment). |
2 participants
Currently, upon running the bot locally, it shows the code in text format. But it would be convenient if shown as QR like this:
Thats' the FIX ✅.
Summary by CodeRabbit