feat: display profile avatar image with terminal graphics protocols#23
feat: display profile avatar image with terminal graphics protocols#23unhappychoice merged 2 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds terminal image support: new Changes
Sequence DiagramsequenceDiagram
participant User
participant Main as Main (CLI)
participant Steam as SteamClient
participant Image as ImageDisplay
participant Term as Terminal
User->>Main: run with --image [--image-protocol]
Main->>Steam: fetch_stats(user_id)
Steam-->>Main: SteamStats { avatar_url, ... }
Main->>Image: load_cached_or_download(avatar_url, cache_key)
alt cache hit
Image-->>Main: DynamicImage
else cache miss
Image->>Image: download_image(url)
Image->>Image: save_to_cache
Image-->>Main: DynamicImage
end
Main->>Image: print_image_and_rewind(img, protocol, cols, rows)
Image->>Term: render via chosen protocol (Kitty/iTerm/Sixel/Block)
Term-->>User: image displayed (or ASCII fallback if unsupported)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 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: 6
🤖 Fix all issues with AI agents
In `@Cargo.toml`:
- Around line 27-28: Update the viuer dependency in Cargo.toml from "0.9" to
"0.11.0" (replace the viuer = "0.9" line with viuer = "0.11.0"), then run cargo
update to refresh lockfile and rebuild to surface any API breakages; check any
uses of viuer API (e.g., functions like print_from_file or Config types) and
adapt callsites to match the 0.11.0 API if the compiler reports changes.
In `@src/image_display.rs`:
- Around line 9-12: download_image currently calls reqwest::get which uses a
default client with no timeout; create a dedicated reqwest::Client with a
timeout (e.g., Duration::from_secs(10)) and use client.get(url).send().await
(then .bytes().await) instead of reqwest::get so the request will fail fast on
slow/unresponsive servers; keep the existing Option return pattern by mapping
send/bytes/load_from_memory errors to None and reference the download_image
function when making the change.
- Around line 14-21: The cache key derived from user-controlled Steam
`personaname` can contain path separators and enable path traversal; modify the
code that builds and uses cache keys (e.g., where `format!("avatar_{}.png",
stats.username)` is produced and passed into `load_cached_or_download`,
`save_to_cache`, and `load_from_cache`) to produce a filesystem-safe key: either
sanitize the username by removing/replacing path-significant characters (like
`/`, `\`, `..`, null bytes) and restricting to a safe charset (alphanumeric,
dash, underscore) or compute a deterministic hash (e.g., SHA-256 or SHA-1 hex)
of the username and use that as the filename (e.g., `avatar_<hash>.png`); ensure
the same sanitized/hash logic is used wherever cache keys are generated and
consumed so existing lookup/save works correctly.
- Around line 23-38: Enable viuer's sixel feature in Cargo.toml (set viuer = {
version = "0.9", features = ["sixel"] }) and update print_image (function
print_image in src/image_display.rs) to set ViuerConfig.use_sixel based on the
ImageProtocol (check ImageProtocol::Sixel and ImageProtocol::Auto) so that when
protocol == ImageProtocol::Sixel viuer emits sixel output; keep existing
use_kitty/use_iterm logic and ensure use_sixel is included in the ViuerConfig
initialization.
In `@src/main.rs`:
- Around line 68-73: The current ImageConfig sets enabled and show_avatar from
the same expression, making show_avatar redundant; change show_avatar to use
cli.avatar only so that --image enables artwork (image_enabled = cli.image ||
cli.avatar) while show_avatar = cli.avatar controls whether to render the
avatar. Update the construction of display::ImageConfig (the ImageConfig
initializer in main.rs) to set enabled = image_enabled and show_avatar =
cli.avatar, and ensure display::render still branches on image_config.enabled &&
image_config.show_avatar for avatar vs non-avatar image rendering.
- Around line 172-176: The demo's top_game_ids vector does not match the titles
in top_games, causing incorrect artwork; locate the top_game_ids and top_games
variables (same scope as avatar_url) and replace the incorrect IDs (e.g.,
1514950 which points to Neodash) with the correct Steam app IDs that correspond
to the demo titles (e.g., Coin Push RPG and Deep Rock Galactic: Survivor as
listed in top_games); ensure each entry in top_game_ids maps one-to-one to the
names in top_games so --demo --image shows the right artwork.
🧹 Nitpick comments (3)
src/display.rs (2)
7-11:enabledandshow_avatarare always set to the same value.In
main.rs, both fields are computed ascli.image || cli.avatar, so they're always identical. Theshow_avatarfield is effectively redundant today. If the intent is to support a future mode where images are enabled without showing the avatar (e.g., game artwork only), consider adding a--no-avatarflag or documenting the planned distinction. Otherwise, a singleenabledbool would simplify things.
62-80: Consider parallel image fetching for game artwork.The three game artwork images are fetched sequentially. Since they're independent network requests, fetching them concurrently with
futures::join_allortokio::join!would reduce latency. This is a nice-to-have given only 3 images.♻️ Sketch using concurrent fetching
async fn fetch_game_artwork( game_ids: &[u32], config: &ImageConfig, ) -> Vec<(u32, image::DynamicImage)> { if !config.enabled || game_ids.is_empty() { return Vec::new(); } - let mut images = Vec::new(); - // Fetch artwork for top 3 games to keep output reasonable - for &appid in game_ids.iter().take(3) { - let url = image_display::game_header_image_url(appid); - let cache_key = format!("game_{}.jpg", appid); - if let Some(img) = image_display::load_cached_or_download(&url, &cache_key).await { - images.push((appid, img)); - } - } - images + let futures: Vec<_> = game_ids + .iter() + .take(3) + .map(|&appid| async move { + let url = image_display::game_header_image_url(appid); + let cache_key = format!("game_{}.jpg", appid); + image_display::load_cached_or_download(&url, &cache_key) + .await + .map(|img| (appid, img)) + }) + .collect(); + futures::future::join_all(futures) + .await + .into_iter() + .flatten() + .collect() }src/image_display.rs (1)
57-63:save_to_cachealways re-encodes as PNG regardless of source format.JPEG game header images are re-encoded as PNG, which is lossless and typically much larger. This inflates cache size unnecessarily. Consider saving the original bytes instead, or inferring the format from the cache key extension.
unhappychoice
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/image_display.rs`:
- Around line 30-38: The block branch double-resizes the image: remove the
pre-resize in the ResolvedProtocol::Block branch inside print_image_and_rewind
and instead pass the original image to print_block so print_block alone handles
sizing; specifically delete the term/pixel_w/pixel_h/resized lines and call
print_block(&img, cols) (or the existing image variable) and keep the rest
(term_rows handling and the rewind print) intact so print_block performs the
pixel-to-terminal resizing once.
- Around line 50-55: The iTerm branch is still using the original DynamicImage
(img) instead of the resized RGBA data, causing sizing mismatches with
print_kitty/print_sixel; update the call site to pass the resized image data
(the same rgba used for print_kitty/print_sixel) to print_iterm and adjust
print_iterm's signature/implementation to accept the resized image buffer or
resized DynamicImage (reuse symbols rgba, print_iterm, print_kitty, print_sixel)
so the iTerm OSC sequence renders the constrained size consistently with
rows/cols and the cursor rewind logic.
🧹 Nitpick comments (6)
src/image_display.rs (3)
89-106:MaybeUninit::zeroed().assume_init()is sound here but non-idiomatic.For FFI structs, the idiomatic Rust pattern is to keep the value as
MaybeUninituntil after theioctlcall succeeds, then callassume_init(). Callingassume_init()before theioctlis safe forlibc::winsize(all-zeros is valid), but it hides the intent and won't generalize.
245-267: Kitty protocol sends raw RGBA — consider compressed PNG for large images.
print_kittybase64-encodes the raw RGBA pixel buffer (f=32). For a 340×360 image this is ~490 KB raw / ~654 KB base64. The Kitty protocol also supportsf=100(PNG), which would be significantly smaller and faster to transmit. This matters more for game artwork images which could be larger.
222-241:is_sixel_capable_terminalreturnstruefor iTerm2 viaLC_TERMINAL, but iTerm2 has its own protocol.If
LC_TERMINALcontains "iTerm2" andITERM_SESSION_IDis not set (which is unlikely but possible), the auto-detection would resolve to Sixel instead of the native iTerm protocol. The iTerm-specific inline image protocol is more feature-rich. Consider checking the order or excluding iTerm2 from the sixel heuristic.src/display.rs (2)
26-37: Avatar fetch failure silently falls back to ASCII — consider a brief diagnostic.When
image_display::load_cached_or_downloadreturnsNone(line 35), the code silently falls back to ASCII. For users who explicitly passed--image, a brief stderr message (e.g., "Could not load avatar, falling back to ASCII") would help diagnose configuration or network issues.
42-47: Image print failure also falls back silently.Same as above — if
print_image_and_rewindreturnsNone(line 45), the user gets ASCII without explanation. A short diagnostic on stderr would improve UX when--imageis explicitly requested.src/main.rs (1)
166-169: Hardcoded avatar URL in demo data is fine but will break if the CDN asset is removed.Minor concern — the demo's
avatar_urlpoints to a specific Steam CDN asset. If that URL goes stale,--demo --imagewill silently fall back to ASCII. Consider noting this or using a more stable test image URL.
unhappychoice
force-pushed
the
feat/display-images
branch
2 times, most recently
from
3bbe096 to
e0bfa66
Compare
unhappychoice
force-pushed
the
feat/display-images
branch
from
e0bfa66 to
9062e2b
Compare
1 participant
Summary
Add
--imageflag to display Steam profile avatar using terminal graphics protocols instead of the ASCII logo. Closes #6.Features
--imageflag replaces ASCII logo with profile avatar--image-protocolto select protocol:auto(default),kitty,iterm,sixel~/.cache/steamfetch/images/Supported Protocols
Usage
Implementation
image_displaymodule: Downloads, caches, and renders images via native protocol implementations (no viuer dependency)icy_sixelcrate▄) character rendering with true colorioctl(TIOCGWINSZ)withESC[14tfallback (same approach as fastfetch) for accurate image-to-cell mappingESC[1G ESC[NArewind, thenESC[NCper-line offset for side-by-side layoutSteamStatsmodel extended withavatar_urlfield from Steam API player summarySummary by CodeRabbit
New Features
Documentation
Chores