diff --git a/Cargo.lock b/Cargo.lock index 3893632..a592407 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1551,7 +1551,7 @@ checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" [[package]] name = "retch-cli" -version = "0.6.17" +version = "0.6.18" dependencies = [ "anyhow", "base64", @@ -1572,7 +1572,7 @@ dependencies = [ [[package]] name = "retch-sysinfo" -version = "0.1.52" +version = "0.1.53" dependencies = [ "anyhow", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 4df2a59..ebdb252 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ members = [ [package] name = "retch-cli" -version = "0.6.17" +version = "0.6.18" edition = "2021" authors = ["Ken Tobias"] description = "A fast, feature-rich system information fetcher written in Rust (similar to fastfetch or neofetch)" @@ -21,7 +21,7 @@ name = "retch" path = "src/main.rs" [dependencies] -retch-sysinfo = { path = "crates/sysinfo", version = "=0.1.52" } +retch-sysinfo = { path = "crates/sysinfo", version = "=0.1.53" } clap = { version = "4.6", features = ["derive"] } serde = { version = "1.0", features = ["derive"] } toml = "1.1" diff --git a/NOTES.md b/NOTES.md index f744901..89fb716 100644 --- a/NOTES.md +++ b/NOTES.md @@ -96,7 +96,68 @@ The `retch-sysinfo` crate can be used independently as a library for cross-platf --- -## Current State (v0.6.17) +## Current State (v0.6.18) +- **v0.6.18 — `Packages` without root, Rio detection under `sudo`, and aspect-correct logo + scaling** (`crates/sysinfo/src/packages.rs`, `src/logo.rs`, `src/display.rs`). Three + user-reported defects, all found by diffing a `sudo retch --full` run against a plain one + on corrino (i7-1360P, Fedora 44, Rio). + - **`Packages` appeared only under `sudo`** (bugfix). `detect_packages` opened + `/var/lib/rpm/rpmdb.sqlite` with `rusqlite::Connection::open`, i.e. **read-write**. The + rpmdb is `root:root 0644` inside a root-owned directory, so SQLite cannot create the + journal sidecars it wants and **every query** fails with `attempt to write a readonly + database` — note *query*, not `open()`, which is why the existing `eprintln!` (guarding + only the open) never fired and the field vanished with no diagnostic at all. Plain + `mode=ro` does not help for the same reason; it still needs to touch the directory. Fixed + with `open_with_flags(rpm_db_uri(path), SQLITE_OPEN_READ_ONLY | SQLITE_OPEN_URI | + SQLITE_OPEN_NO_MUTEX)` over a `file:…?immutable=1` URI, which lets SQLite skip locking and + sidecars entirely. Verified end to end: `2509` as an unprivileged user, byte-identical to + what the `sudo` run reported. The query error is now surfaced rather than swallowed, so + the next failure in this path says why. Pure `rpm_db_uri` helper, 2 unit tests. + - **Rio lost graphics support under `sudo`** (bugfix). `supports_kitty`/`supports_iterm2`/ + `supports_sixel` identified Rio **only** by `TERM_PROGRAM`, which is not in sudo's default + `env_keep`, so `sudo retch` silently fell all the way through to Chafa while the same + command as the user used the Kitty protocol. New `is_rio_terminal()` also accepts + `TERM=rio`/`xterm-rio` — `TERM` *is* preserved by sudo — and all three checks route through + it. 3 unit tests, including a negative case pinning that `rioja` is not matched. + - **Test-isolation defect fixed in the same change, same class as #155/v0.6.2:** + `test_supports_iterm2_heuristics` guarded only `TERM_PROGRAM`, so once `supports_iterm2` + began reading `TERM` the *host's* value leaked in and its negative assertions failed on a + Rio box while passing on CI and everywhere else. It now guards and clears `TERM` too. + - **The Kitty logo was stretched ~3× vertically** (bugfix). `print_graphical_logo` emitted a + hardcoded `c=26,r=10`, and Kitty **forces** an image into the `c`×`r` rectangle — it does + not preserve aspect ratio when both are given. Five of the assets are wide horizontal + lockups (`fedora.png` is 384×108, i.e. 3.56:1; also arch/nixos/ubuntu/tux), so they were + squashed into a roughly 1:1 cell box. Compounding it, `display.rs` separately assumed a + fixed **40**-column width for layout while `graphical_logo_height_lines` derived the row + count a third way — three inconsistent answers for one footprint. Fixed with a single pure + `fit_logo_cells(img_w, img_h, cell_w, cell_h, max_cols, max_rows) -> LogoFit` that fits the + image in the cell box preserving aspect (in pixels, so non-square cells are handled), now + used by **all three** protocol emitters *and* by `plan_layout`. iTerm2 additionally passes + an explicit cell `width` alongside `preserveAspectRatio=1`; Sixel resizes to the same box + rather than a fixed 240×200. 6 unit tests. + - **Computing `c` and `r` correctly is not sufficient — Kitty must be given only *one* of + them.** Cells are indivisible, so the rounded rectangle is never exactly the image's + aspect, and Kitty scales each axis independently to fill whatever rectangle it is given. + Measured in a PTY with real pixel dimensions (169×47 cells, 22×51 px): passing both + correct values still left a **9%** vertical stretch. `LogoFit::width_limited` records + which dimension the image touches first and `kitty_placement_spec` emits just that one + (`c=45` for Fedora), letting Kitty derive the other — **0.0% aspect error**, verified the + same way. The layout's `div_ceil` reservation (6 rows for a 5.46-row draw) still covers + it, which is the safe direction. + - **Chafa logo box widened 28→45 columns** (`LOGO_MAX_COLS`), height cap unchanged at 10 + (`LOGO_MAX_ROWS`). Chafa fits *within* the box preserving aspect, so a narrow box caps a + wide image's height long before the row cap does: at 28 columns the Fedora logo collapsed + to **4 rows** of symbols and was unreadable; at 45 it renders **7**. Both chafa call sites + now share `chafa_size_arg()`. **The side-by-side threshold is unaffected** — the text column floors + at 45 and 45 + 45 = 90 ≤ 95, so a full-width logo still sits beside the text at the 95-col + cutoff; pinned by a new `plan_layout` test at both 95 and 169 columns. + - Assets deliberately **not** changed: cropping the wide lockups to their square icon halves + would render larger still, but it is a content decision and was declined in favour of the + layout-only fix. + - New §6b documents the privilege-dependent fields in both directions (root-only + `phys-mem`/btrfs snapshots; user-only `editor`/`desktop`/`wm`), mirrored in `README.md` + and a new `PRIVILEGES` section in `docs/retch.1.md`. + - `retch-sysinfo` → `0.1.53` (library behaviour change); `retch-cli` → `0.6.18`. Patch bump. - **v0.6.17 — Disabled Claude Code Review on GitHub Actions CI** (`.github/workflows/claude-code-review.yml`). Disabled the `pull_request` trigger and set `if: false` on the `claude-review` job in `claude-code-review.yml`. `retch-cli` → `0.6.17`. Patch bump. - **v0.6.16 — Graphic logo size reduction and controlled info line wrapping** (`src/display.rs`, `src/logo.rs`, `crates/sysinfo/src/audio.rs`). @@ -889,6 +950,38 @@ load-average equivalent), `editor` (env-only `$VISUAL`/`$EDITOR`), conhost `term --- +## 6b. Privilege-dependent fields (Linux) — what `sudo retch` changes, both directions + +Running retch under `sudo` does not simply add fields: it adds some and **removes others**, +because `sudo`'s default `env_reset` strips most of the environment. Diffing a `sudo --full` +run against a plain one is therefore not a fair before/after, and the differences below are +expected behaviour, not bugs. (`Packages` used to be on this list and no longer is — see the +v0.6.18 entry.) + +**Only available as root** +- **`phys-mem`** — reads `/sys/firmware/dmi/tables/DMI`, mode `0400 root`, via `dmidecode`. + There is no unprivileged source for per-DIMM type/capacity/speed on Linux, so the field is + omitted rather than guessed. (Windows reads SMBIOS natively and needs no elevation.) +- **`btrfs` snapshot count** — `btrfs subvolume list -s` requires root. Deliberately + **omitted rather than shown as `0`** when it cannot be read, so "couldn't check" is never + mistaken for "no snapshots"; the label/subvolume/space part of the field still renders. + +**Only available as the logged-in user** (lost under `sudo`, since `env_reset` drops them) +- **`editor`** — `$VISUAL` / `$EDITOR`. +- **`desktop`**, **`wm`** — `XDG_CURRENT_DESKTOP` / `XDG_SESSION_DESKTOP` / `GDMSESSION`. +- **Logo protocol selection** — `TERM_PROGRAM` is not in sudo's `env_keep`. This used to + silently downgrade Rio from the Kitty graphics protocol to Chafa; since v0.6.18 the Rio + check also consults `TERM` (`xterm-rio`), which sudo *does* preserve. Any other terminal + identified solely by `TERM_PROGRAM` still degrades under sudo by design — it is a heuristic + over an environment sudo is entitled to clear. + +**Rule of thumb for this class of bug:** if a field is missing only for the unprivileged user, +check whether the underlying source is genuinely root-only before adding an elevation note — +`Packages` looked exactly like a permissions limit for a long time and was in fact a fixable +SQLite open-mode defect. + +--- + ## 7. Major Achievements ### v0.6.1 - Fix Windows Camera (scanners) + Users (=0) bugs (July 13, 2026) diff --git a/README.md b/README.md index f48b6b3..9aab185 100644 --- a/README.md +++ b/README.md @@ -149,6 +149,19 @@ Show help: retch --help ``` +### Running under `sudo` + +`sudo retch` is not simply "retch with more fields" — it trades one set for another, because +`sudo`'s default `env_reset` clears most of the environment: + +| | Field | +|---|---| +| **Root only** | `phys-mem` (reads the DMI tables, mode `0400 root`); the snapshot count in `btrfs` (`btrfs subvolume list -s`) | +| **User only** | `editor` (`$VISUAL`/`$EDITOR`), `desktop` and `wm` (`XDG_CURRENT_DESKTOP` and friends) | + +Everything else is identical either way, so run retch normally unless you specifically want +the DIMM breakdown or btrfs snapshot counts. + ## Shell Completions Generate completion scripts for your shell: @@ -235,6 +248,8 @@ separator_color = "bright_black" # Ordered list of system information fields to display # Note: "phys-mem" requires root (sudo) on Linux to read DMI memory tables. On Windows, reads the SMBIOS table natively (no PowerShell). +# Note: "btrfs" snapshot counts require root on Linux; the count is omitted (not shown as 0) when it can't be read. +# Note: "editor", "desktop" and "wm" read environment variables, so they are absent under `sudo` (env_reset). # Note: "phys-disk" on Windows uses native storage IOCTLs (no PowerShell, no admin). # Note: "weather" requires network access; shown in full mode only by default. # Note: "domain-search" queries resolvectl; shown in full mode only by default. diff --git a/crates/sysinfo/Cargo.toml b/crates/sysinfo/Cargo.toml index ef3ab72..934d8c1 100644 --- a/crates/sysinfo/Cargo.toml +++ b/crates/sysinfo/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "retch-sysinfo" -version = "0.1.52" +version = "0.1.53" edition = "2021" authors = ["Ken Tobias"] description = "System information gathering library for retch" diff --git a/crates/sysinfo/src/packages.rs b/crates/sysinfo/src/packages.rs index 49ab6df..faed7be 100644 --- a/crates/sysinfo/src/packages.rs +++ b/crates/sysinfo/src/packages.rs @@ -6,6 +6,21 @@ //! Supports Pacman (Arch), Dpkg (Debian), XBPS (Void), RPM (Fedora/RHEL) on Linux, //! Homebrew (Formulae and Casks) and MacPorts on macOS, and Scoop/Chocolatey on Windows. +/// Builds the SQLite URI used to read the RPM database without write access. +/// +/// `/var/lib/rpm/rpmdb.sqlite` is owned by root (mode 0644) inside a root-owned directory, +/// so an unprivileged process cannot create the journal sidecar files SQLite wants — and +/// SQLite reports that as `attempt to write a readonly database` on the **query**, not on +/// `open()`. Plain `mode=ro` is not enough for the same reason: it still needs to touch the +/// directory. `immutable=1` promises SQLite the file will not change while it is open, which +/// lets it skip locking and sidecars entirely, so the count succeeds as a normal user. +/// +/// This is why `Packages` previously appeared only under `sudo`. +#[cfg(any(not(any(target_os = "macos", target_os = "windows")), test))] +fn rpm_db_uri(path: &str) -> String { + format!("file:{path}?immutable=1") +} + pub(crate) fn detect_packages() -> Option { #[cfg(target_os = "macos")] { @@ -105,18 +120,27 @@ pub(crate) fn detect_packages() -> Option { let rpm_db = "/var/lib/rpm/rpmdb.sqlite"; if std::path::Path::new(rpm_db).exists() { - match rusqlite::Connection::open(rpm_db) { + use rusqlite::OpenFlags; + let flags = OpenFlags::SQLITE_OPEN_READ_ONLY + | OpenFlags::SQLITE_OPEN_URI + | OpenFlags::SQLITE_OPEN_NO_MUTEX; + match rusqlite::Connection::open_with_flags(rpm_db_uri(rpm_db), flags) { Ok(conn) => { - if let Ok(count) = conn.query_row("SELECT COUNT(*) FROM Packages", [], |row| { + match conn.query_row("SELECT COUNT(*) FROM Packages", [], |row| { row.get::<_, i64>(0) }) { - if count > 0 { - return Some(count as usize); + Ok(count) if count > 0 => return Some(count as usize), + Ok(_) => {} + // Surfaced rather than swallowed: the read-only-database failure this + // URI exists to prevent used to land here and vanish silently, so the + // field simply disappeared with no clue why. + Err(e) => { + eprintln!("warning: failed to query RPM database at {rpm_db}: {e}"); } } } Err(e) => { - eprintln!("warning: failed to open RPM database at {}: {}", rpm_db, e); + eprintln!("warning: failed to open RPM database at {rpm_db}: {e}"); } } } @@ -124,3 +148,27 @@ pub(crate) fn detect_packages() -> Option { None } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_rpm_db_uri_requests_immutable() { + // `immutable=1` is the load-bearing part: without it an unprivileged read of the + // root-owned rpmdb fails with "attempt to write a readonly database". + assert_eq!( + rpm_db_uri("/var/lib/rpm/rpmdb.sqlite"), + "file:/var/lib/rpm/rpmdb.sqlite?immutable=1" + ); + } + + #[test] + fn test_rpm_db_uri_is_a_file_uri() { + // The `file:` scheme is what makes SQLITE_OPEN_URI parse the query string at all; + // a bare path would silently ignore `immutable=1`. + let uri = rpm_db_uri("/tmp/some.sqlite"); + assert!(uri.starts_with("file:")); + assert!(uri.contains("?immutable=1")); + } +} diff --git a/docs/retch.1 b/docs/retch.1 index ac3896c..452c8d8 100644 --- a/docs/retch.1 +++ b/docs/retch.1 @@ -1,4 +1,4 @@ -.TH "RETCH" "1" "August 2026" "retch 0.6.17" "System Information Fetcher" +.TH "RETCH" "1" "August 2026" "retch 0.6.18" "System Information Fetcher" .SH "NAME" .PP @@ -441,6 +441,37 @@ Logos are automatically suppressed when stdout is not a terminal (e\.g\. when pi .PP Use `\f[CR]\-\-ascii\-only\fP` to force text\-only output\. Use `\f[CR]\-\-no\-logo\fP` to suppress the logo unconditionally\. +.SH "PRIVILEGES" +.PP +Running \fBretch\fP under \fBsudo\fP(8) exchanges one set of fields for another rather than +simply adding to them, because sudo's default `\f[CR]env_reset\fP` clears most of the environment\. +.PP +Available only as root: +.RS +.Bl +.IP \(bu 4 +`\f[CR]phys\-mem\fP` \-\- the per\-DIMM breakdown reads the DMI tables (`\f[CR]/sys/firmware/dmi/tables/DMI\fP`, mode 0400)\. +.El +.Bl +.IP \(bu 4 +`\f[CR]btrfs\fP` \-\- the snapshot count requires `\f[CR]btrfs subvolume list \-s\fP`\. It is omitted, never shown as zero, when it cannot be read\. +.El +.RE +.PP +Available only as the logged\-in user: +.RS +.Bl +.IP \(bu 4 +`\f[CR]editor\fP` \-\- read from `\f[CR]$VISUAL\fP` / `\f[CR]$EDITOR\fP`\. +.El +.Bl +.IP \(bu 4 +`\f[CR]desktop\fP`, `\f[CR]wm\fP` \-\- read from `\f[CR]XDG_CURRENT_DESKTOP\fP`, `\f[CR]XDG_SESSION_DESKTOP\fP` and `\f[CR]GDMSESSION\fP`\. +.El +.RE +.PP +All other fields are identical either way\. + .SH "EXIT STATUS" .PP \fBretch\fP exits with status 0 on success, and non\-zero on error\. diff --git a/docs/retch.1.md b/docs/retch.1.md index 936be5e..657d429 100644 --- a/docs/retch.1.md +++ b/docs/retch.1.md @@ -187,6 +187,23 @@ retch supports both ASCII and graphical logos. Use `--ascii-only` to force text-only output. Use `--no-logo` to suppress the logo unconditionally. +# PRIVILEGES + +Running **retch** under **sudo**(8) exchanges one set of fields for another rather than +simply adding to them, because sudo's default `env_reset` clears most of the environment. + +Available only as root: + +- `phys-mem` — the per-DIMM breakdown reads the DMI tables (`/sys/firmware/dmi/tables/DMI`, mode 0400). +- `btrfs` — the snapshot count requires `btrfs subvolume list -s`. It is omitted, never shown as zero, when it cannot be read. + +Available only as the logged-in user: + +- `editor` — read from `$VISUAL` / `$EDITOR`. +- `desktop`, `wm` — read from `XDG_CURRENT_DESKTOP`, `XDG_SESSION_DESKTOP` and `GDMSESSION`. + +All other fields are identical either way. + # EXIT STATUS **retch** exits with status 0 on success, and non-zero on error. diff --git a/src/display.rs b/src/display.rs index 2054f8b..07a924a 100644 --- a/src/display.rs +++ b/src/display.rs @@ -58,8 +58,9 @@ struct LayoutPlan { /// /// This is logo-type-agnostic: `logo_height`/`logo_width` are supplied by the caller from the /// active logo, so it works identically for ASCII art, Chafa (both rendered as text lines), -/// and the graphical image protocols (Kitty/iTerm2/Sixel, whose height is their pixel-derived -/// row count and whose width is the fixed image column). +/// and the graphical image protocols (Kitty/iTerm2/Sixel, whose cell footprint comes from +/// [`logo::fit_logo_cells`] — the *same* call the emitters use to size the image, so the +/// reserved area and the drawn area cannot disagree). /// /// `info_widths` are the ANSI-stripped visible widths of the info lines, in render order. fn plan_layout( @@ -632,9 +633,9 @@ pub fn display(info: &SystemInfo, cli: &Cli, config: &Config) -> anyhow::Result< // Setup logo representation enum ActiveLogo { Lines(Vec), - Kitty(Vec, usize), // bytes, height_lines - Iterm2(Vec, usize), - Sixel(Vec, usize), + Kitty(Vec, usize, usize), // bytes, cols, rows + Iterm2(Vec, usize, usize), + Sixel(Vec, usize, usize), None, } @@ -689,14 +690,14 @@ pub fn display(info: &SystemInfo, cli: &Cli, config: &Config) -> anyhow::Result< if !resolved && logo::supports_kitty() { if let Some(path) = &user_logo { if let Ok(bytes) = std::fs::read(path) { - let h = graphical_logo_height_lines(&bytes); - active_logo = ActiveLogo::Kitty(bytes, h); + let (cols, rows) = graphical_logo_cells(&bytes); + active_logo = ActiveLogo::Kitty(bytes, cols, rows); resolved = true; } } else if let Some(distro) = &distro_hint { if let Some(bytes) = logo::get_embedded_logo(Some(distro)) { - let h = graphical_logo_height_lines(bytes); - active_logo = ActiveLogo::Kitty(bytes.to_vec(), h); + let (cols, rows) = graphical_logo_cells(bytes); + active_logo = ActiveLogo::Kitty(bytes.to_vec(), cols, rows); resolved = true; } } @@ -707,14 +708,14 @@ pub fn display(info: &SystemInfo, cli: &Cli, config: &Config) -> anyhow::Result< if !resolved && logo::supports_iterm2() { if let Some(path) = &user_logo { if let Ok(bytes) = std::fs::read(path) { - let h = graphical_logo_height_lines(&bytes); - active_logo = ActiveLogo::Iterm2(bytes, h); + let (cols, rows) = graphical_logo_cells(&bytes); + active_logo = ActiveLogo::Iterm2(bytes, cols, rows); resolved = true; } } else if let Some(distro) = &distro_hint { if let Some(bytes) = logo::get_embedded_logo(Some(distro)) { - let h = graphical_logo_height_lines(bytes); - active_logo = ActiveLogo::Iterm2(bytes.to_vec(), h); + let (cols, rows) = graphical_logo_cells(bytes); + active_logo = ActiveLogo::Iterm2(bytes.to_vec(), cols, rows); resolved = true; } } @@ -725,14 +726,14 @@ pub fn display(info: &SystemInfo, cli: &Cli, config: &Config) -> anyhow::Result< if !resolved && logo::supports_sixel() { if let Some(path) = &user_logo { if let Ok(bytes) = std::fs::read(path) { - let h = graphical_logo_height_lines(&bytes); - active_logo = ActiveLogo::Sixel(bytes, h); + let (cols, rows) = graphical_logo_cells(&bytes); + active_logo = ActiveLogo::Sixel(bytes, cols, rows); resolved = true; } } else if let Some(distro) = &distro_hint { if let Some(bytes) = logo::get_embedded_logo(Some(distro)) { - let h = graphical_logo_height_lines(bytes); - active_logo = ActiveLogo::Sixel(bytes.to_vec(), h); + let (cols, rows) = graphical_logo_cells(bytes); + active_logo = ActiveLogo::Sixel(bytes.to_vec(), cols, rows); resolved = true; } } @@ -801,7 +802,9 @@ pub fn display(info: &SystemInfo, cli: &Cli, config: &Config) -> anyhow::Result< .max() .unwrap_or(0), ), - ActiveLogo::Kitty(_, h) | ActiveLogo::Iterm2(_, h) | ActiveLogo::Sixel(_, h) => (*h, 40), + ActiveLogo::Kitty(_, cols, rows) + | ActiveLogo::Iterm2(_, cols, rows) + | ActiveLogo::Sixel(_, cols, rows) => (*rows, *cols), ActiveLogo::None => (0, 0), }; @@ -851,7 +854,7 @@ pub fn display(info: &SystemInfo, cli: &Cli, config: &Config) -> anyhow::Result< println!("{}{}{}", info_line, padding, logo_line); } } - ActiveLogo::Kitty(bytes, logo_rows) => { + ActiveLogo::Kitty(bytes, _, logo_rows) => { render_graphical_side_by_side( text_column_width, &formatted_info_lines, @@ -859,7 +862,7 @@ pub fn display(info: &SystemInfo, cli: &Cli, config: &Config) -> anyhow::Result< || logo::print_graphical_logo(&bytes), ); } - ActiveLogo::Iterm2(bytes, logo_rows) => { + ActiveLogo::Iterm2(bytes, _, logo_rows) => { render_graphical_side_by_side( text_column_width, &formatted_info_lines, @@ -867,7 +870,7 @@ pub fn display(info: &SystemInfo, cli: &Cli, config: &Config) -> anyhow::Result< || logo::print_iterm2_logo(&bytes), ); } - ActiveLogo::Sixel(bytes, logo_rows) => { + ActiveLogo::Sixel(bytes, _, logo_rows) => { render_graphical_side_by_side( text_column_width, &formatted_info_lines, @@ -890,15 +893,15 @@ pub fn display(info: &SystemInfo, cli: &Cli, config: &Config) -> anyhow::Result< } println!(); } - ActiveLogo::Kitty(bytes, _) => { + ActiveLogo::Kitty(bytes, _, _) => { logo::print_graphical_logo(&bytes); println!(); } - ActiveLogo::Iterm2(bytes, _) => { + ActiveLogo::Iterm2(bytes, _, _) => { logo::print_iterm2_logo(&bytes); println!(); } - ActiveLogo::Sixel(bytes, _) => { + ActiveLogo::Sixel(bytes, _, _) => { logo::print_sixel_logo(&bytes); println!(); } @@ -1008,32 +1011,21 @@ fn format_uptime(uptime: &str) -> String { parts.join(" ") } -/// Returns the height in terminal rows a graphical logo image will occupy. +/// Returns the `(columns, rows)` a graphical logo image will occupy on this terminal. /// -/// Uses TIOCGWINSZ pixel dimensions on Unix to get the real cell height. -/// Falls back to 20px per cell when the terminal doesn't report pixel dims. +/// Delegates to [`logo::logo_cells_for`], which is also what the Kitty/iTerm2/Sixel emitters +/// use to size the image itself — so the footprint reserved by [`plan_layout`] and the +/// footprint actually drawn are the same numbers by construction. They used to be computed +/// independently (rows here from the pixel height, width hardcoded to 40, and the Kitty +/// escape hardcoding a third answer), which is how the logo ended up stretched *and* +/// mis-positioned. #[cfg(feature = "graphics")] -fn graphical_logo_height_lines(bytes: &[u8]) -> usize { - let img_h = image::load_from_memory(bytes) - .map(|img| img.height() as usize) - .unwrap_or(200); - let cell_h = terminal_cell_height_px(); - let rows = img_h.div_ceil(cell_h); - rows.min(10) -} - -/// Returns the terminal cell height in pixels via TIOCGWINSZ, or 20 as fallback. -fn terminal_cell_height_px() -> usize { - #[cfg(unix)] - { - use std::mem::MaybeUninit; - let mut ws: libc::winsize = unsafe { MaybeUninit::zeroed().assume_init() }; - let ret = unsafe { libc::ioctl(libc::STDOUT_FILENO, libc::TIOCGWINSZ, &mut ws) }; - if ret == 0 && ws.ws_row > 0 && ws.ws_ypixel > 0 { - return ws.ws_ypixel as usize / ws.ws_row as usize; - } - } - 20 +fn graphical_logo_cells(bytes: &[u8]) -> (usize, usize) { + let (img_w, img_h) = image::load_from_memory(bytes) + .map(|img| (img.width(), img.height())) + .unwrap_or((0, 0)); + let fit = logo::logo_cells_for(img_w, img_h); + (fit.cols, fit.rows) } #[cfg(test)] @@ -1131,6 +1123,24 @@ mod tests { assert_eq!(p.text_column_width, 45); // max(10+4, 45) } + #[test] + fn test_layout_widened_logo_box_still_fits_at_the_side_by_side_threshold() { + // The logo cell box grew from 28 to `logo::LOGO_MAX_COLS` (45) so wide-aspect logos get + // enough rows to stay legible. That must not cost the side-by-side layout at the 95-col + // threshold: the text column floors at 45, and 45 + 45 = 90 <= 95. + let p = plan_layout(&[10; 25], 10, logo::LOGO_MAX_COLS, 95, true); + assert!( + p.side_by_side, + "a full-width logo must still sit beside the text at 95 columns" + ); + assert!(p.text_column_width + logo::LOGO_MAX_COLS <= 95); + + // And a wide terminal is unaffected — the text column still reaches its 65 cap. + let wide = plan_layout(&[120; 25], 10, logo::LOGO_MAX_COLS, 169, true); + assert!(wide.side_by_side); + assert_eq!(wide.text_column_width, 65); + } + #[test] fn test_layout_logo_taller_than_text() { // Fewer info lines than logo rows: all lines are beside the logo (no panic on slice). diff --git a/src/logo.rs b/src/logo.rs index 17d7778..a30572d 100644 --- a/src/logo.rs +++ b/src/logo.rs @@ -215,6 +215,23 @@ pub fn get_distro_logo_lines(distro: Option<&str>) -> Vec { .collect() } +/// Returns true when the running terminal is Rio. +/// +/// Checks `TERM` as well as `TERM_PROGRAM`. `TERM_PROGRAM` alone is not sufficient: it is not +/// in sudo's default `env_keep` list, so `sudo retch` lost Rio's graphics support entirely and +/// silently fell all the way back to Chafa. `TERM` **is** preserved by sudo (`xterm-rio` here), +/// and the same gap affects any launcher that starts retch with a trimmed environment. +fn is_rio_terminal() -> bool { + if let Ok(term) = std::env::var("TERM") { + if term == "rio" || term.starts_with("xterm-rio") { + return true; + } + } + std::env::var("TERM_PROGRAM") + .map(|t| t == "rio") + .unwrap_or(false) +} + /// Checks if the terminal supports the Kitty inline image protocol. pub fn supports_kitty() -> bool { std::env::var("TERM") @@ -223,19 +240,17 @@ pub fn supports_kitty() -> bool { || std::env::var("TERMINAL_EMULATOR") .map(|t| t == "iterm-kitty" || t == "iTerm.app") .unwrap_or(false) - || std::env::var("TERM_PROGRAM") - .map(|t| t == "rio") - .unwrap_or(false) + || is_rio_terminal() } /// Checks if the terminal supports the iTerm2 inline image protocol. pub fn supports_iterm2() -> bool { if let Ok(prog) = std::env::var("TERM_PROGRAM") { - if prog == "iTerm.app" || prog == "WezTerm" || prog == "rio" { + if prog == "iTerm.app" || prog == "WezTerm" { return true; } } - false + is_rio_terminal() } /// Checks if the terminal supports Sixel graphics (heuristic based on environment). @@ -248,7 +263,7 @@ pub fn supports_sixel() -> bool { } if let Ok(prog) = std::env::var("TERM_PROGRAM") { - if prog == "WezTerm" || prog == "iTerm.app" || prog == "rio" { + if prog == "WezTerm" || prog == "iTerm.app" { return true; } } @@ -257,7 +272,142 @@ pub fn supports_sixel() -> bool { return true; } - false + is_rio_terminal() +} + +/// Maximum width, in terminal columns, that a rendered logo may occupy. +/// +/// Widened from 28 so that wide-aspect assets (the horizontal lockups — `fedora.png` is +/// 384×108, i.e. 3.56:1 — plus arch/nixos/ubuntu/tux) get enough rows to stay legible: a +/// logo is fitted *inside* this box preserving aspect, so a narrow box caps a wide image's +/// height long before [`LOGO_MAX_ROWS`] does. At 28 columns the Fedora logo collapsed to 4 +/// rows of Chafa symbols and was unreadable. +pub const LOGO_MAX_COLS: usize = 45; + +/// Maximum height, in terminal rows, that a rendered logo may occupy. +pub const LOGO_MAX_ROWS: usize = 10; + +/// Fits an image into a cell box **preserving its aspect ratio**, returning a [`LogoFit`]. +/// +/// Both the image and the box are converted to pixels (via the terminal's cell dimensions) +/// so the terminal's non-square cells are accounted for — a 2:1 image in 1:2 cells is 4 +/// columns per row, not 2. The result is the smallest cell rectangle that contains the +/// scaled image, clamped to `1..=max`. +/// +/// This is the single source of truth for a graphical logo's footprint: the same values feed +/// the protocol escape (so the terminal does not stretch the image) and `plan_layout` (so the +/// text column is placed against the logo's real width). Previously the Kitty path hardcoded +/// `c=26,r=10` — which *forces* the image into that rectangle, ignoring aspect entirely, so +/// the 3.56:1 Fedora logo was squashed into a roughly 1:1 box and rendered ~3× too tall — +/// while the layout separately assumed a fixed 40-column width. +pub fn fit_logo_cells( + img_w: u32, + img_h: u32, + cell_w: usize, + cell_h: usize, + max_cols: usize, + max_rows: usize, +) -> LogoFit { + let (max_cols, max_rows) = (max_cols.max(1), max_rows.max(1)); + + // Degenerate inputs: fall back to the full box rather than dividing by zero. + if img_w == 0 || img_h == 0 || cell_w == 0 || cell_h == 0 { + return LogoFit { + cols: max_cols, + rows: max_rows, + width_limited: true, + }; + } + + let (img_w, img_h) = (u64::from(img_w), u64::from(img_h)); + let box_w = (max_cols * cell_w) as u64; + let box_h = (max_rows * cell_h) as u64; + + // Compare box_w/img_w against box_h/img_h without floating point: whichever ratio is + // smaller is the limiting dimension. + let width_limited = box_w * img_h <= box_h * img_w; + let (disp_w, disp_h) = if width_limited { + (box_w, box_w * img_h / img_w) // wide image: touches the sides first + } else { + (box_h * img_w / img_h, box_h) // tall image: touches top and bottom first + }; + + LogoFit { + // `div_ceil` so the reservation is never *smaller* than what gets drawn: an extra + // blank row or column is harmless, an overlapping one corrupts the layout. + cols: (disp_w as usize).div_ceil(cell_w).clamp(1, max_cols), + rows: (disp_h as usize).div_ceil(cell_h).clamp(1, max_rows), + width_limited, + } +} + +/// The cell footprint of a logo, plus which dimension of the box it touches first. +/// +/// `width_limited` matters only to the Kitty emitter — see [`kitty_placement_spec`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct LogoFit { + /// Width in terminal columns. + pub cols: usize, + /// Height in terminal rows. + pub rows: usize, + /// True when the image reaches the box's width before its height (a wide image). + pub width_limited: bool, +} + +/// Builds the Kitty graphics-protocol placement keys (`c=` / `r=`) for a fitted logo. +/// +/// Deliberately emits **only the limiting dimension**. Kitty derives the other from the +/// image's real aspect ratio, whereas specifying both makes it scale each axis independently +/// to fill the rectangle exactly — and because cells are indivisible, the rounded rectangle is +/// never quite the image's aspect, so passing both leaves a residual stretch even when the +/// numbers are computed correctly (measured at 9% for the Fedora logo). Passing one leaves +/// none. +/// +/// The reservation [`plan_layout`](crate::display) makes is `div_ceil`-rounded and therefore +/// always covers what Kitty then draws. +pub fn kitty_placement_spec(fit: LogoFit) -> String { + if fit.width_limited { + format!("c={}", fit.cols) + } else { + format!("r={}", fit.rows) + } +} + +/// Returns the terminal cell size in pixels as `(width, height)` via `TIOCGWINSZ`. +/// +/// Falls back to a 10×20 cell — the conventional default — when the terminal does not report +/// pixel dimensions (tmux, many terminals, and any non-TTY stdout). +pub fn terminal_cell_size_px() -> (usize, usize) { + #[cfg(unix)] + { + use std::mem::MaybeUninit; + let mut ws: libc::winsize = unsafe { MaybeUninit::zeroed().assume_init() }; + // SAFETY: `ws` is a live, zeroed `winsize` and TIOCGWINSZ writes exactly that type. + let ret = unsafe { libc::ioctl(libc::STDOUT_FILENO, libc::TIOCGWINSZ, &mut ws) }; + if ret == 0 && ws.ws_row > 0 && ws.ws_col > 0 && ws.ws_xpixel > 0 && ws.ws_ypixel > 0 { + return ( + ws.ws_xpixel as usize / ws.ws_col as usize, + ws.ws_ypixel as usize / ws.ws_row as usize, + ); + } + } + (10, 20) +} + +/// Returns the cell footprint an image will occupy on the current terminal. +/// +/// Thin wrapper pairing [`terminal_cell_size_px`] with the pure [`fit_logo_cells`]. +pub fn logo_cells_for(img_w: u32, img_h: u32) -> LogoFit { + let (cell_w, cell_h) = terminal_cell_size_px(); + fit_logo_cells(img_w, img_h, cell_w, cell_h, LOGO_MAX_COLS, LOGO_MAX_ROWS) +} + +/// The `--size WxH` argument passed to `chafa`, derived from the shared logo cell box. +/// +/// Chafa fits the image *inside* this box preserving aspect ratio, so this is a maximum in +/// both dimensions rather than a target — a wide logo comes back short, a tall one narrow. +pub fn chafa_size_arg() -> String { + format!("{LOGO_MAX_COLS}x{LOGO_MAX_ROWS}") } static CHAFA_SUPPORTS_PROBE: std::sync::OnceLock = std::sync::OnceLock::new(); @@ -298,7 +448,7 @@ pub fn print_with_chafa(path: &std::path::Path) -> bool { cmd.arg("--format") .arg("symbols") .arg("--size") - .arg("28x10"); + .arg(chafa_size_arg()); if chafa_supports_probe() { cmd.arg("--probe").arg("off"); @@ -328,7 +478,7 @@ pub fn get_chafa_logo_lines(path: &std::path::Path) -> Option> { cmd.arg("--format") .arg("symbols") .arg("--size") - .arg("28x10"); + .arg(chafa_size_arg()); if chafa_supports_probe() { cmd.arg("--probe").arg("off"); @@ -435,11 +585,28 @@ pub fn print_distro_logo_with_ascii(distro: Option<&str>, ascii_only: bool, chaf #[cfg(feature = "graphics")] pub fn print_iterm2_logo(image_data: &[u8]) { use base64::Engine; + + let (width, height) = image::load_from_memory(image_data) + .map(|img| (img.width(), img.height())) + .unwrap_or((0, 0)); + let encoded = base64::engine::general_purpose::STANDARD.encode(image_data); - print!( - "\x1b]1337;File=inline=1;height=10;preserveAspectRatio=1:{}\x07", - encoded - ); + + // `width`/`height` are in character cells here; `preserveAspectRatio=1` makes them a + // bounding box rather than a target, so the image is never distorted. Passing both (not + // just `height`) keeps the drawn footprint inside the width `plan_layout` reserved. + if width > 0 && height > 0 { + let fit = logo_cells_for(width, height); + print!( + "\x1b]1337;File=inline=1;width={};height={};preserveAspectRatio=1:{}\x07", + fit.cols, fit.rows, encoded + ); + } else { + print!( + "\x1b]1337;File=inline=1;height={};preserveAspectRatio=1:{}\x07", + LOGO_MAX_ROWS, encoded + ); + } println!(); // iTerm2 typically needs a newline after the logo } @@ -471,9 +638,14 @@ pub fn print_graphical_logo(image_data: &[u8]) { let encoded = base64::engine::general_purpose::STANDARD.encode(image_data); if width > 0 && height > 0 { + // Kitty *forces* the image into whatever placement rectangle it is given, so the + // spec carries only the limiting dimension and lets Kitty derive the other from the + // image's aspect ratio. The old hardcoded `c=26,r=10` is what squashed the 3.56:1 + // Fedora logo into a roughly 1:1 box. + let spec = kitty_placement_spec(logo_cells_for(width, height)); println!( - "\x1b_Gf=100,s={},v={},c=26,r=10,a=T;{}\x1b\\", - width, height, encoded + "\x1b_Gf=100,s={},v={},{},a=T;{}\x1b\\", + width, height, spec, encoded ); } else { println!("\x1b_Gf=100,a=T;{}", encoded); @@ -484,7 +656,16 @@ pub fn print_graphical_logo(image_data: &[u8]) { #[cfg(feature = "graphics")] pub fn print_sixel_logo(image_data: &[u8]) { if let Ok(img) = image::load_from_memory(image_data) { - let resized = img.resize(240, 200, image::imageops::FilterType::Triangle); + // Size the sixel to the same cell box the layout reserved, in pixels. `resize` already + // preserves aspect ratio (it fits within the box), so this only ever shrinks the image + // to the footprint `plan_layout` was told about. + let fit = logo_cells_for(img.width(), img.height()); + let (cell_w, cell_h) = terminal_cell_size_px(); + let resized = img.resize( + (fit.cols * cell_w) as u32, + (fit.rows * cell_h) as u32, + image::imageops::FilterType::Triangle, + ); let rgba = resized.to_rgba8(); let (width, height) = rgba.dimensions(); print_sixel_rgba(rgba.as_raw(), width, height); @@ -649,7 +830,11 @@ mod tests { #[test] fn test_supports_iterm2_heuristics() { - let _guard = EnvGuard::new(&["TERM_PROGRAM"]); + // TERM must be guarded and cleared as well as TERM_PROGRAM: `supports_iterm2` consults + // it via `is_rio_terminal`, so without this the *host's* TERM leaks in and the negative + // assertions below fail on a Rio box while passing everywhere else. + let _guard = EnvGuard::new(&["TERM", "TERM_PROGRAM"]); + std::env::remove_var("TERM"); // Test TERM_PROGRAM=iTerm.app std::env::set_var("TERM_PROGRAM", "iTerm.app"); @@ -801,4 +986,114 @@ mod tests { assert!(!garuda.is_empty()); assert!(garuda.iter().any(|line| line.contains("888:8898898"))); } + + // ── Rio detection (TERM as well as TERM_PROGRAM) ────────────────────────── + + #[test] + fn test_rio_detected_from_term_when_term_program_is_absent() { + // The sudo case: `env_reset` keeps TERM but drops TERM_PROGRAM, which used to cost + // Rio all graphics support and fall through to Chafa. + let _guard = EnvGuard::new(&["TERM", "TERMINAL_EMULATOR", "TERM_PROGRAM"]); + std::env::remove_var("TERM_PROGRAM"); + std::env::remove_var("TERMINAL_EMULATOR"); + std::env::set_var("TERM", "xterm-rio"); + + assert!(is_rio_terminal()); + assert!(supports_kitty()); + assert!(supports_iterm2()); + assert!(supports_sixel()); + } + + #[test] + fn test_rio_still_detected_from_term_program() { + // The pre-existing path must keep working when TERM says nothing useful. + let _guard = EnvGuard::new(&["TERM", "TERMINAL_EMULATOR", "TERM_PROGRAM"]); + std::env::remove_var("TERMINAL_EMULATOR"); + std::env::set_var("TERM", "xterm-256color"); + std::env::set_var("TERM_PROGRAM", "rio"); + + assert!(is_rio_terminal()); + assert!(supports_kitty()); + } + + #[test] + fn test_non_rio_term_is_not_matched() { + // Guard against a loose substring match: these must not be taken for Rio. + let _guard = EnvGuard::new(&["TERM", "TERMINAL_EMULATOR", "TERM_PROGRAM"]); + std::env::remove_var("TERM_PROGRAM"); + std::env::remove_var("TERMINAL_EMULATOR"); + for term in ["xterm-256color", "screen", "linux", "rioja"] { + std::env::set_var("TERM", term); + assert!(!is_rio_terminal(), "{term} should not be detected as Rio"); + } + } + + // ── fit_logo_cells ──────────────────────────────────────────────────────── + + #[test] + fn test_fit_logo_cells_preserves_aspect_for_wide_image() { + // fedora.png is 384x108 (3.56:1). In 10x20px cells a 45x10 box is 450x200px, so the + // image is width-limited: 450px wide -> 450*108/384 = 126px tall -> 7 rows. + // The old hardcoded c=26,r=10 forced it into 260x200px, a ~3x vertical stretch. + let fit = fit_logo_cells(384, 108, 10, 20, 45, 10); + assert_eq!(fit.cols, 45); + assert_eq!(fit.rows, 7); + assert!(fit.width_limited); + // A wide image is pinned by width, so Kitty is told the width and derives the height. + assert_eq!(kitty_placement_spec(fit), "c=45"); + } + + #[test] + fn test_fit_logo_cells_preserves_aspect_for_tall_image() { + // debian.png is 291x384 (0.76:1) — height-limited, so it must not claim the full width. + let fit = fit_logo_cells(291, 384, 10, 20, 45, 10); + assert_eq!(fit.rows, 10); + assert!( + fit.cols < 45, + "tall image should not fill the width, got {}", + fit.cols + ); + assert!(!fit.width_limited); + assert_eq!(kitty_placement_spec(fit), "r=10"); + } + + #[test] + fn test_fit_logo_cells_accounts_for_non_square_cells() { + // A square image in 1:2 cells must come back twice as wide as it is tall, otherwise + // it renders visibly squashed. Same image, square cells, stays square. + let fit = fit_logo_cells(256, 256, 10, 20, 45, 10); + assert_eq!((fit.cols, fit.rows), (20, 10)); + let sq = fit_logo_cells(256, 256, 10, 10, 45, 10); + assert_eq!((sq.cols, sq.rows), (10, 10)); + } + + #[test] + fn test_fit_logo_cells_never_exceeds_the_box() { + // Whatever the aspect, the result must fit the budget plan_layout was given. + for (w, h) in [(384, 108), (291, 384), (256, 256), (4000, 3), (3, 4000)] { + let fit = fit_logo_cells(w, h, 10, 20, 45, 10); + assert!((1..=45).contains(&fit.cols), "{w}x{h} -> {} cols", fit.cols); + assert!((1..=10).contains(&fit.rows), "{w}x{h} -> {} rows", fit.rows); + } + } + + #[test] + fn test_fit_logo_cells_handles_degenerate_input() { + // Unreadable image dimensions or a terminal reporting zero-size cells must not panic + // or divide by zero. + for fit in [ + fit_logo_cells(0, 0, 10, 20, 45, 10), + fit_logo_cells(384, 108, 0, 20, 45, 10), + fit_logo_cells(384, 108, 10, 0, 45, 10), + ] { + assert_eq!((fit.cols, fit.rows), (45, 10)); + } + } + + #[test] + fn test_chafa_size_arg_matches_the_shared_box() { + // Chafa and the graphical protocols must budget the same footprint. + assert_eq!(chafa_size_arg(), format!("{LOGO_MAX_COLS}x{LOGO_MAX_ROWS}")); + assert_eq!(chafa_size_arg(), "45x10"); + } }