From ebda3c51afb5b8692547c1de7b303cf158e3ae16 Mon Sep 17 00:00:00 2001 From: jasonwitty Date: Tue, 18 Aug 2026 22:47:03 -0700 Subject: [PATCH] fix(ui): keep the important text visible on narrow terminals (#38) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three panes painted two independent pieces of text onto one row with nothing reserving space between them, so below roughly 105 columns the right-hand piece was simply drawn over the tail of the left one: socktop — host: cachyos-gaming | 🔒✗ TLS | (a: about⏱ 500ms metrics | 2000ms The width arithmetic used str::len(), a byte count, so the emoji in these strings overstated their width and left orphaned glyphs at the right edge as well. The process table had the same problem in a different form: it handed the layout solver a fixed, over-constrained column set, so a narrow pane crushed the percentage-sized Name column to nothing while fixed-width PID and Mem % kept their full width — losing the one field that identifies a process. Add ui::fit (measure in terminal columns, truncate on character boundaries, pick the richest wording that fits) and give each pane a priority ladder: - Header: drop the key hints, then the TLS/token badges, then the "socktop — host:" prefix, then the metrics/procs words, and only then shorten the hostname. Hostname and intervals are what survive longest. - CPU pane: drop the "CPU Temp:" label, then the now:/avg: labels, then the average, then the temperature's decimal, and the temperature itself last — a thermal warning outranks a second decimal place. - Process table: Name is unconditional; CPU %, then Mem, then PID, then Mem % are added as the pane widens, so Mem % is the first to go and Name the last. Also fixes sort-header clicks, which resolved against a Layout that omitted the column spacing the Table renders with, so a click landed off by up to four columns. Covered by a test that clicks each label where it is actually drawn. Co-authored-by: Claude Opus 5 --- Cargo.lock | 1 + Cargo.toml | 1 + socktop/Cargo.toml | 1 + socktop/src/app.rs | 39 ++-- socktop/src/ui/cpu.rs | 204 ++++++++++++++--- socktop/src/ui/fit.rs | 133 +++++++++++ socktop/src/ui/header.rs | 234 ++++++++++++++++++-- socktop/src/ui/mod.rs | 1 + socktop/src/ui/processes.rs | 429 +++++++++++++++++++++++++++++++++--- 9 files changed, 946 insertions(+), 97 deletions(-) create mode 100644 socktop/src/ui/fit.rs diff --git a/Cargo.lock b/Cargo.lock index 234db01..b48e4a5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2426,6 +2426,7 @@ dependencies = [ "sysinfo", "tempfile", "tokio", + "unicode-width", "url", ] diff --git a/Cargo.toml b/Cargo.toml index 3733559..d0a7bfe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ sysinfo = "0.37" # CLI UI ratatui = "0.30" crossterm = "0.29" +unicode-width = "0.2" # web server (remote-agent) axum = { version = "0.7", features = ["ws"] } diff --git a/socktop/Cargo.toml b/socktop/Cargo.toml index e7368a8..0bf3419 100644 --- a/socktop/Cargo.toml +++ b/socktop/Cargo.toml @@ -20,6 +20,7 @@ serde_json = { workspace = true } url = { workspace = true } ratatui = { workspace = true } crossterm = { workspace = true } +unicode-width = { workspace = true } anyhow = { workspace = true } dirs-next = { workspace = true } sysinfo = { workspace = true } diff --git a/socktop/src/app.rs b/socktop/src/app.rs index 68551b7..881cf8c 100644 --- a/socktop/src/app.rs +++ b/socktop/src/app.rs @@ -36,7 +36,7 @@ use crate::ui::processes::{ use crate::ui::{ disks::draw_disks, gpu::{draw_gpu, draw_gpu_compact}, - header::{build_header_intervals, build_header_title, draw_header}, + header::{HeaderState, build_header, draw_header}, mem::draw_mem, net::draw_net_spark, swap::draw_swap, @@ -153,9 +153,8 @@ pub struct App { // Cached title strings — only rebuilt when source values change so the // diff renderer can suppress redraws on idle frames. header_title: String, - header_title_key: (String, bool, bool), header_intervals_text: String, - header_intervals_key: (u128, u128), + header_key: (String, bool, bool, u128, u128, u16), net_dl_title: String, net_dl_key: (u64, u64), net_ul_title: String, @@ -236,9 +235,8 @@ impl App { has_token: false, force_compact: false, header_title: String::new(), - header_title_key: (String::new(), false, false), header_intervals_text: String::new(), - header_intervals_key: (u128::MAX, u128::MAX), + header_key: (String::new(), false, false, u128::MAX, u128::MAX, u16::MAX), net_dl_title: String::new(), net_dl_key: (u64::MAX, u64::MAX), net_ul_title: String::new(), @@ -1278,27 +1276,30 @@ impl App { let l = self.layout(area); // Header — refresh cached strings only when their inputs change so the - // ratatui diff renderer can suppress repaints on idle frames. + // ratatui diff renderer can suppress repaints on idle frames. The wording now + // depends on the row width too, so that is part of the key. { let hostname = self.last_metrics.as_ref().map(|mm| mm.hostname.as_str()); + let state = HeaderState { + hostname, + is_tls: self.is_tls, + has_token: self.has_token, + metrics_ms: self.metrics_interval.as_millis(), + procs_ms: self.procs_interval.as_millis(), + }; let key = ( hostname.unwrap_or("").to_string(), self.is_tls, self.has_token, + state.metrics_ms, + state.procs_ms, + l.header.width, ); - if self.header_title_key != key { - self.header_title = build_header_title(hostname, self.is_tls, self.has_token); - self.header_title_key = key; - } - - let intervals_key = ( - self.metrics_interval.as_millis(), - self.procs_interval.as_millis(), - ); - if self.header_intervals_key != intervals_key { - self.header_intervals_text = - build_header_intervals(intervals_key.0, intervals_key.1); - self.header_intervals_key = intervals_key; + if self.header_key != key { + let (title, intervals) = build_header(state, l.header.width); + self.header_title = title; + self.header_intervals_text = intervals; + self.header_key = key; } } draw_header(f, l.header, &self.header_title, &self.header_intervals_text); diff --git a/socktop/src/ui/cpu.rs b/socktop/src/ui/cpu.rs index 1857b98..e78add0 100644 --- a/socktop/src/ui/cpu.rs +++ b/socktop/src/ui/cpu.rs @@ -14,6 +14,10 @@ use ratatui::{ use crate::history::PerCoreHistory; use crate::types::Metrics; +use crate::ui::fit::{cols, pick_pair}; + +/// Columns kept clear between the CPU title and the temperature readout. +const TITLE_GAP: u16 = 2; /// State for dragging the scrollbar thumb #[derive(Clone, Copy, Debug, Default)] @@ -248,29 +252,12 @@ pub fn draw_cpu_avg_graph( hist_sum as f64 / hist.len() as f64 }; - let title = if let Some(mm) = m { - format!("CPU (now: {:>5.1}% | avg: {:>5.1}%)", mm.cpu_total, avg_cpu) - } else { - "CPU avg".into() - }; - - // Build the top-right info (CPU temp and polling intervals) - let top_right_info = if let Some(mm) = m { - mm.cpu_temp_c - .map(|t| { - let icon = if t < 50.0 { - "😎" - } else if t < 85.0 { - "⚠️" - } else { - "🔥" - }; - format!("CPU Temp: {t:.1}°C {icon}") - }) - .unwrap_or_else(|| "CPU Temp: N/A".into()) - } else { - String::new() - }; + let (title, top_right_info) = cpu_title_for_width( + m.map(|mm| mm.cpu_total), + avg_cpu, + m.and_then(|mm| mm.cpu_temp_c), + area.width, + ); // Hand a slice directly to Sparkline. `make_contiguous` is amortized cheap // for our usage pattern (cap'd 600-element ring updated at 2 Hz) and lets @@ -286,12 +273,14 @@ pub fn draw_cpu_avg_graph( .style(Style::default().fg(Color::Cyan)); f.render_widget(spark, area); - // Render the top-right info as text overlay in the top-right corner + // Temperature overlays the top border, right-aligned inside the corner. The title + // above is sized so the two cannot collide. if !top_right_info.is_empty() { + let w = cols(&top_right_info); let info_area = Rect { - x: area.x + area.width.saturating_sub(top_right_info.len() as u16 + 2), + x: area.x + area.width.saturating_sub(w + 1), y: area.y, - width: top_right_info.len() as u16 + 1, + width: w, height: 1, }; let info_line = Line::from(Span::raw(top_right_info)); @@ -299,6 +288,67 @@ pub fn draw_cpu_avg_graph( } } +/// Health glyph for a CPU temperature. +fn temp_icon(t: f32) -> &'static str { + if t < 50.0 { + "😎" + } else if t < 85.0 { + "⚠️" + } else { + "🔥" + } +} + +/// Chooses the CPU pane's title and its right-aligned temperature readout for a pane +/// `width` columns wide. +/// +/// Both are painted onto the pane's top border, so without a shared budget the +/// temperature simply overwrites the tail of the title on a narrow pane. Detail is given +/// up in this order: the `CPU Temp:` label, then the `now:`/`avg:` labels, then the +/// average reading, then the decimal on the temperature, and only last the temperature +/// itself — the readings are what the pane is for, but a thermal warning is worth more +/// than a second decimal place. +fn cpu_title_for_width( + cpu_now: Option, + avg_cpu: f64, + temp_c: Option, + width: u16, +) -> (String, String) { + let Some(now) = cpu_now else { + return ("CPU avg".into(), String::new()); + }; + + // Two borders, plus a column of breathing room at each end of the title. + let budget = width.saturating_sub(4); + + let labelled = format!("CPU (now: {now:>5.1}% | avg: {avg_cpu:>5.1}%)"); + let bare = format!("CPU ({now:.1}% | {avg_cpu:.1}%)"); + let now_only = format!("CPU ({now:.1}%)"); + + let (temp_labelled, temp_plain, temp_coarse) = match temp_c { + Some(t) => { + let icon = temp_icon(t); + ( + format!("CPU Temp: {t:.1}°C {icon}"), + format!("{t:.1}°C {icon}"), + format!("{t:.0}°C {icon}"), + ) + } + None => ("CPU Temp: N/A".into(), "N/A".into(), "N/A".into()), + }; + + let ladder = [ + (labelled.as_str(), temp_labelled.as_str()), + (labelled.as_str(), temp_plain.as_str()), + (bare.as_str(), temp_plain.as_str()), + (bare.as_str(), temp_coarse.as_str()), + (now_only.as_str(), temp_coarse.as_str()), + (now_only.as_str(), ""), + ]; + let (title, temp) = pick_pair(budget, TITLE_GAP, &ladder); + (title.to_string(), temp.to_string()) +} + /// Draws the per-core CPU bars with sparklines and trends. pub fn draw_per_core_bars( f: &mut ratatui::Frame<'_>, @@ -428,6 +478,108 @@ pub fn draw_per_core_bars( } } +#[cfg(test)] +mod title_tests { + use super::*; + + /// The defect this replaces: the temperature was painted over the title's tail on a + /// narrow pane. Whatever the width, the two must fit side by side on the border. + #[test] + fn title_and_temperature_never_overlap() { + for width in 0..=200u16 { + let (title, temp) = cpu_title_for_width(Some(3.4), 12.7, Some(43.0), width); + let budget = width.saturating_sub(4); + if temp.is_empty() { + continue; + } + assert!( + cols(&title) + cols(&temp) + TITLE_GAP <= budget, + "width {width}: {title:?} + {temp:?} do not fit in {budget} columns" + ); + } + } + + /// The current CPU reading is the one thing the pane must always show. + #[test] + fn the_current_reading_always_survives() { + for width in 20..=200u16 { + let (title, _) = cpu_title_for_width(Some(3.4), 12.7, Some(43.0), width); + assert!( + title.contains("3.4"), + "width {width}: lost the reading ({title:?})" + ); + } + } + + /// The ladder from the design: temp label, then now/avg labels, then the average, + /// then the temperature's decimal, then the temperature. + #[test] + fn detail_is_dropped_in_priority_order() { + let at = |w| cpu_title_for_width(Some(0.7), 1.3, Some(43.0), w); + + let (title, temp) = at(80); + assert_eq!(title, "CPU (now: 0.7% | avg: 1.3%)"); + assert_eq!(temp, "CPU Temp: 43.0°C 😎"); + + // The "CPU Temp:" label goes first; the readings keep their labels. + let (title, temp) = at(50); + assert_eq!(title, "CPU (now: 0.7% | avg: 1.3%)"); + assert_eq!(temp, "43.0°C 😎"); + + // Then the now:/avg: labels. + let (title, temp) = at(40); + assert_eq!(title, "CPU (0.7% | 1.3%)"); + assert_eq!(temp, "43.0°C 😎"); + + // Then the temperature's decimal. + let (title, temp) = at(31); + assert_eq!(title, "CPU (0.7% | 1.3%)"); + assert_eq!(temp, "43°C 😎"); + + // Then the average reading. + let (title, temp) = at(26); + assert_eq!(title, "CPU (0.7%)"); + assert_eq!(temp, "43°C 😎"); + + // Last of all, the temperature itself. + let (title, temp) = at(15); + assert_eq!(title, "CPU (0.7%)"); + assert_eq!(temp, ""); + } + + /// A hot CPU has to stay visible as a warning, so the glyph rides along with the + /// reading at every tier that shows a temperature at all. + #[test] + fn the_thermal_glyph_tracks_the_temperature() { + for (t, icon) in [(43.0, "😎"), (70.0, "⚠️"), (92.0, "🔥")] { + for width in 26..=80u16 { + let (_, temp) = cpu_title_for_width(Some(0.7), 1.3, Some(t), width); + assert!( + temp.contains(icon), + "width {width} at {t}°C: expected {icon} in {temp:?}" + ); + } + } + } + + /// An agent that reports no temperature must not leave a stray label behind. + #[test] + fn a_missing_temperature_degrades_to_nothing() { + let (_, temp) = cpu_title_for_width(Some(0.7), 1.3, None, 80); + assert_eq!(temp, "CPU Temp: N/A"); + let (_, temp) = cpu_title_for_width(Some(0.7), 1.3, None, 14); + assert_eq!(temp, ""); + } + + /// Before the first payload arrives there are no readings to show. + #[test] + fn no_metrics_yet_shows_the_placeholder() { + let (title, temp) = cpu_title_for_width(None, 0.0, None, 80); + assert_eq!(title, "CPU avg"); + assert!(temp.is_empty()); + } +} + #[cfg(test)] mod render_tests { use super::*; diff --git a/socktop/src/ui/fit.rs b/socktop/src/ui/fit.rs new file mode 100644 index 0000000..75011af --- /dev/null +++ b/socktop/src/ui/fit.rs @@ -0,0 +1,133 @@ +//! Fitting text to the columns actually available. +//! +//! Several panes paint two independent pieces of text onto one row — a left title and a +//! right-aligned readout. Nothing reserves space for the right piece, so on a narrow +//! terminal the right one is simply painted over the tail of the left one and the title +//! is clobbered mid-word. The helpers here let a caller measure in real terminal columns +//! and pick the richest wording that still fits, so the two never overlap. +//! +//! Note that `str::len()` is a byte count and must not be used for this: `⏱` is three +//! bytes wide but one column, and `🔒` is four bytes but two columns. + +use unicode_width::UnicodeWidthStr; + +/// Terminal columns `s` occupies, saturating at `u16::MAX`. +pub fn cols(s: &str) -> u16 { + UnicodeWidthStr::width(s).min(u16::MAX as usize) as u16 +} + +/// Shortens `s` to at most `max` columns, marking the cut with `…`. +/// +/// Cuts on character boundaries and accounts for wide characters, so the result never +/// exceeds `max` columns and never splits a multi-byte character. +pub fn truncate_cols(s: &str, max: u16) -> String { + if cols(s) <= max { + return s.to_string(); + } + if max == 0 { + return String::new(); + } + // Reserve one column for the ellipsis. + let budget = max.saturating_sub(1); + let mut used = 0u16; + let mut out = String::new(); + for ch in s.chars() { + let w = cols(ch.encode_utf8(&mut [0u8; 4])); + if used + w > budget { + break; + } + used += w; + out.push(ch); + } + out.push('…'); + out +} + +/// Picks the first (richest) candidate pair that fits side by side in `width` columns +/// with at least `gap` columns between them. +/// +/// Candidates are ordered most- to least-detailed; the last one is the floor and is +/// returned even if it does not fit, so callers always get something to render. +pub fn pick_pair<'a>( + width: u16, + gap: u16, + candidates: &[(&'a str, &'a str)], +) -> (&'a str, &'a str) { + let fits = |left: &str, right: &str| { + let needed = cols(left) + .saturating_add(cols(right)) + .saturating_add(if right.is_empty() { 0 } else { gap }); + needed <= width + }; + for &(left, right) in candidates { + if fits(left, right) { + return (left, right); + } + } + candidates.last().copied().unwrap_or(("", "")) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The bug these helpers exist to prevent: byte length overstates the width of the + /// glyphs socktop puts in its header, which is what pushed the right-hand text into + /// the title in the first place. + #[test] + fn cols_counts_columns_not_bytes() { + assert_eq!(cols("abc"), 3); + // Stopwatch: 3 bytes, 1 column. + assert_eq!("⏱".len(), 3); + assert_eq!(cols("⏱"), 1); + // Lock: 4 bytes, 2 columns. + assert_eq!("🔒".len(), 4); + assert_eq!(cols("🔒"), 2); + assert_eq!(cols("⏱ 500ms metrics | 2000ms procs"), 30); + } + + #[test] + fn truncate_respects_the_column_budget() { + assert_eq!(truncate_cols("cachyos-gaming", 20), "cachyos-gaming"); + assert_eq!(truncate_cols("cachyos-gaming", 14), "cachyos-gaming"); + assert_eq!(truncate_cols("cachyos-gaming", 10), "cachyos-g…"); + assert_eq!(cols(&truncate_cols("cachyos-gaming", 10)), 10); + assert_eq!(truncate_cols("cachyos-gaming", 1), "…"); + assert_eq!(truncate_cols("cachyos-gaming", 0), ""); + } + + /// Truncation must never land mid-character or overrun the budget on wide glyphs. + #[test] + fn truncate_handles_wide_and_multibyte_characters() { + for max in 0..12u16 { + let out = truncate_cols("🔒🔒🔒 TLS", max); + assert!(cols(&out) <= max, "{out:?} exceeds {max} columns"); + assert!(out.chars().all(|c| c != '\u{fffd}'), "{out:?} split a char"); + } + // A wide glyph that cannot fit beside the ellipsis is dropped whole. + assert_eq!(truncate_cols("🔒ab", 2), "…"); + } + + #[test] + fn pick_pair_takes_the_richest_that_fits() { + let candidates = [ + ("full left text", "full right text"), + ("left text", "right text"), + ("left", "right"), + ]; + assert_eq!(pick_pair(80, 2, &candidates), candidates[0]); + assert_eq!(pick_pair(24, 2, &candidates), candidates[1]); + assert_eq!(pick_pair(12, 2, &candidates), candidates[2]); + // Below the floor the last candidate is still returned. + assert_eq!(pick_pair(1, 2, &candidates), candidates[2]); + } + + /// The gap is what keeps the two pieces from touching; it must not be charged when + /// there is no right-hand piece to separate. + #[test] + fn pick_pair_only_charges_the_gap_when_both_sides_are_present() { + let candidates = [("0123456789", "x"), ("0123456789", "")]; + assert_eq!(pick_pair(11, 2, &candidates), candidates[1]); + assert_eq!(pick_pair(13, 2, &candidates), candidates[0]); + } +} diff --git a/socktop/src/ui/header.rs b/socktop/src/ui/header.rs index c76f7b9..dd1e909 100644 --- a/socktop/src/ui/header.rs +++ b/socktop/src/ui/header.rs @@ -1,44 +1,232 @@ -//! Top header with hostname and CPU temperature indicator. +//! Top header with hostname, connection status and polling intervals. +//! +//! The row carries two pieces of text — session identity on the left, polling intervals +//! on the right — and both matter. Rather than let the right one overwrite the left when +//! they no longer both fit, the header drops detail in priority order: the hostname and +//! the intervals are what survive longest, because they are what tells you *which* host +//! you are looking at and how fresh the numbers are. +use crate::ui::fit::{cols, pick_pair, truncate_cols}; use ratatui::{ layout::Rect, text::{Line, Span}, widgets::{Block, Borders, Paragraph}, }; -/// Build the header's left-side title from session state. Callers cache the -/// returned String and only rebuild it when one of the inputs changes. -pub fn build_header_title(hostname: Option<&str>, is_tls: bool, has_token: bool) -> String { - let base = match hostname { - Some(h) => format!("socktop — host: {h}"), - None => "socktop — connecting...".into(), - }; - let tls_txt = if is_tls { "🔒 TLS" } else { "🔒✗ TLS" }; - let mut parts = vec![base, tls_txt.into()]; - if has_token { - parts.push("🔑 token".into()); - } - parts.push("(a: about, h: help, q: quit)".into()); - parts.join(" | ") +/// Columns kept clear between the left and right halves. +const GAP: u16 = 2; +/// Never shorten the hostname below this before dropping the intervals instead. +const HOSTNAME_FLOOR: u16 = 8; + +/// Session state the header renders. +#[derive(Clone, Copy)] +pub struct HeaderState<'a> { + pub hostname: Option<&'a str>, + pub is_tls: bool, + pub has_token: bool, + pub metrics_ms: u128, + pub procs_ms: u128, } -/// Build the right-side polling interval text. Callers cache this string. -pub fn build_header_intervals(metrics_ms: u128, procs_ms: u128) -> String { - format!("⏱ {metrics_ms}ms metrics | {procs_ms}ms procs") +/// Builds the left and right halves of the header for a row `width` columns wide. +/// +/// Detail is dropped in this order as the row narrows: the key hints, then the TLS/token +/// badges, then the `socktop — host:` prefix (leaving the bare hostname), then the +/// `metrics`/`procs` words, and only then is the hostname itself shortened. The two +/// halves are always sized to sit side by side, so neither can paint over the other. +/// +/// Callers cache the result and rebuild it only when the state or the width changes. +pub fn build_header(state: HeaderState<'_>, width: u16) -> (String, String) { + let host = state.hostname.unwrap_or("connecting..."); + let tls = if state.is_tls { + "🔒 TLS" + } else { + "🔒✗ TLS" + }; + let badges = if state.has_token { + format!("{tls} | 🔑 token") + } else { + tls.to_string() + }; + + let named = format!("socktop — host: {host}"); + let with_badges = format!("{named} | {badges}"); + let with_keys = format!("{with_badges} | (a: about, h: help, q: quit)"); + + let intervals = format!( + "⏱ {}ms metrics | {}ms procs", + state.metrics_ms, state.procs_ms + ); + let intervals_short = format!("⏱ {}ms | {}ms", state.metrics_ms, state.procs_ms); + + // Richest first. The bare hostname is reached before the intervals lose their + // labels, and the hostname is only shortened once nothing else is left to give. + let ladder = [ + (with_keys.as_str(), intervals.as_str()), + (with_badges.as_str(), intervals.as_str()), + (named.as_str(), intervals.as_str()), + (host, intervals.as_str()), + (host, intervals_short.as_str()), + ]; + let (left, right) = pick_pair(width, GAP, &ladder); + if cols(left) + cols(right) + GAP <= width { + return (left.to_string(), right.to_string()); + } + + // Past the floor of the ladder: shorten the hostname, and give up the intervals only + // if even a stub of a hostname will not fit beside them. + let room = width + .saturating_sub(cols(&intervals_short)) + .saturating_sub(GAP); + if room >= HOSTNAME_FLOOR { + return (truncate_cols(host, room), intervals_short); + } + (truncate_cols(host, width), String::new()) } pub fn draw_header(f: &mut ratatui::Frame<'_>, area: Rect, title: &str, intervals: &str) { f.render_widget(Block::default().title(title).borders(Borders::BOTTOM), area); - let intervals_width = intervals.len() as u16; - if area.width > intervals_width + 2 { + if intervals.is_empty() { + return; + } + let intervals_width = cols(intervals); + if area.width >= intervals_width { let right_area = Rect { - x: area.x + area.width.saturating_sub(intervals_width + 1), + x: area.x + area.width - intervals_width, y: area.y, width: intervals_width, height: 1, }; - let intervals_line = Line::from(Span::raw(intervals)); - f.render_widget(Paragraph::new(intervals_line), right_area); + f.render_widget(Paragraph::new(Line::from(Span::raw(intervals))), right_area); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn state(hostname: Option<&str>) -> HeaderState<'_> { + HeaderState { + hostname, + is_tls: false, + has_token: false, + metrics_ms: 500, + procs_ms: 2000, + } + } + + /// The defect this replaces: the two halves were painted independently, so below + /// ~105 columns the right half landed on top of the title. Whatever the width, they + /// must now fit side by side. + #[test] + fn halves_never_overlap_at_any_width() { + for width in 0..=200u16 { + let (left, right) = build_header(state(Some("cachyos-gaming")), width); + let used = cols(&left) + cols(&right); + if right.is_empty() { + assert!(cols(&left) <= width, "width {width}: {left:?} overflows"); + } else { + assert!( + used + GAP <= width, + "width {width}: {left:?} + {right:?} = {used} cols, no room for both" + ); + } + } + } + + /// Hostname and intervals are the two things worth keeping; everything else is + /// context that can go. + #[test] + fn hostname_and_intervals_survive_longest() { + for width in 34..=200u16 { + let (left, right) = build_header(state(Some("cachyos-gaming")), width); + assert!( + left.contains("cachyos-gaming"), + "width {width}: lost the hostname ({left:?})" + ); + assert!( + right.contains("500ms") && right.contains("2000ms"), + "width {width}: lost the intervals ({right:?})" + ); + } + } + + /// The ladder from the design: key hints, then badges, then the prefix, then the + /// interval labels, then the hostname itself. + #[test] + fn detail_is_dropped_in_priority_order() { + let s = state(Some("cachyos-gaming")); + + let (left, right) = build_header(s, 120); + assert_eq!( + left, + "socktop — host: cachyos-gaming | 🔒✗ TLS | (a: about, h: help, q: quit)" + ); + assert_eq!(right, "⏱ 500ms metrics | 2000ms procs"); + + // Key hints go first. + let (left, _) = build_header(s, 80); + assert_eq!(left, "socktop — host: cachyos-gaming | 🔒✗ TLS"); + + // Then the badges. + let (left, _) = build_header(s, 70); + assert_eq!(left, "socktop — host: cachyos-gaming"); + + // Then the prefix, leaving the bare hostname. + let (left, right) = build_header(s, 50); + assert_eq!(left, "cachyos-gaming"); + assert_eq!(right, "⏱ 500ms metrics | 2000ms procs"); + + // Then the interval labels. + let (left, right) = build_header(s, 34); + assert_eq!(left, "cachyos-gaming"); + assert_eq!(right, "⏱ 500ms | 2000ms"); + + // Only then is the hostname itself shortened. + // 30 columns - 16 for the short intervals - 2 gap leaves 12 for the hostname. + let (left, right) = build_header(s, 30); + assert_eq!(left, "cachyos-gam…"); + assert_eq!(right, "⏱ 500ms | 2000ms"); + } + + /// A long hostname must not push the intervals off the row. + #[test] + fn a_long_hostname_is_shortened_rather_than_winning_the_row() { + let long = "a-very-long-hostname-that-will-not-fit-anywhere"; + for width in 30..=100u16 { + let (left, right) = build_header(state(Some(long)), width); + assert!(!right.is_empty(), "width {width}: intervals were dropped"); + assert!(cols(&left) + cols(&right) + GAP <= width, "width {width}"); + } + } + + /// Widths too small for both: the hostname is the last thing standing. + #[test] + fn hostname_is_the_final_survivor() { + let (left, right) = build_header(state(Some("cachyos-gaming")), 20); + assert!(right.is_empty(), "intervals should have been dropped"); + assert!(!left.is_empty()); + assert!(cols(&left) <= 20); + } + + #[test] + fn tls_and_token_badges_appear_when_there_is_room() { + let s = HeaderState { + hostname: Some("host"), + is_tls: true, + has_token: true, + metrics_ms: 500, + procs_ms: 2000, + }; + let (left, _) = build_header(s, 200); + assert!(left.contains("🔒 TLS"), "{left}"); + assert!(left.contains("🔑 token"), "{left}"); + } + + #[test] + fn a_missing_hostname_reads_as_connecting() { + let (left, _) = build_header(state(None), 120); + assert!(left.contains("connecting"), "{left}"); } } diff --git a/socktop/src/ui/mod.rs b/socktop/src/ui/mod.rs index 7c1536a..81da61d 100644 --- a/socktop/src/ui/mod.rs +++ b/socktop/src/ui/mod.rs @@ -2,6 +2,7 @@ pub mod cpu; pub mod disks; +pub mod fit; pub mod gpu; pub mod header; pub mod layout; diff --git a/socktop/src/ui/processes.rs b/socktop/src/ui/processes.rs index e1519d9..33fd194 100644 --- a/socktop/src/ui/processes.rs +++ b/socktop/src/ui/processes.rs @@ -134,14 +134,83 @@ pub fn rebuild_row_cache(metrics: &Metrics, out: &mut Vec) -> f32 { peak } -// Keep the original header widths here so drawing and hit-testing match. -const COLS: [Constraint; 5] = [ - Constraint::Length(8), // PID - Constraint::Percentage(40), // Name - Constraint::Length(8), // CPU % - Constraint::Length(12), // Mem - Constraint::Length(8), // Mem % -]; +const PID_W: u16 = 8; +const CPU_W: u16 = 8; +const MEM_W: u16 = 12; +const MEM_PCT_W: u16 = 8; +/// Columns the Name field needs to identify anything. Every other column is only added +/// once Name already has this much, so Name can no longer be squeezed to nothing. +const NAME_MIN_W: u16 = 8; +/// `Table::column_spacing`. +const COL_SPACING: u16 = 1; + +/// Which process columns fit in the pane, and where they sit. +/// +/// The table used to hand the layout solver a fixed, over-constrained set, so on a narrow +/// pane the solver crushed the percentage-sized Name column to nothing while the fixed +/// PID and Mem % columns kept their full width — losing the one field that identifies the +/// process while keeping the ones that do not. +/// +/// Columns are now added in priority order as the pane widens, so they are shed in +/// reverse as it narrows: Name is unconditional, then CPU %, then Mem, then PID, and +/// Mem % last (it is derivable from Mem, so it is the least costly to lose). +/// +/// Both the draw path and the header-click hit-testing build this from the same width, so +/// a sort click always lands on the column the user can actually see. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct ProcColumns { + pub pid: bool, + pub cpu: bool, + pub mem: bool, + pub mem_pct: bool, +} + +impl ProcColumns { + pub fn for_width(width: u16) -> Self { + // Each tier is the previous one plus a column and the gap before it. + let with_cpu = NAME_MIN_W + COL_SPACING + CPU_W; + let with_mem = with_cpu + COL_SPACING + MEM_W; + let with_pid = with_mem + COL_SPACING + PID_W; + let with_mem_pct = with_pid + COL_SPACING + MEM_PCT_W; + Self { + cpu: width >= with_cpu, + mem: width >= with_mem, + pid: width >= with_pid, + mem_pct: width >= with_mem_pct, + } + } + + /// Column constraints in render order. Name takes whatever the others leave. + pub fn constraints(&self) -> Vec { + let mut c = Vec::with_capacity(5); + if self.pid { + c.push(Constraint::Length(PID_W)); + } + c.push(Constraint::Fill(1)); // Name + if self.cpu { + c.push(Constraint::Length(CPU_W)); + } + if self.mem { + c.push(Constraint::Length(MEM_W)); + } + if self.mem_pct { + c.push(Constraint::Length(MEM_PCT_W)); + } + c + } + + /// Position of the CPU % column, which is clickable to sort. `None` when too narrow + /// to render it. + pub fn cpu_index(&self) -> Option { + self.cpu.then(|| 1 + usize::from(self.pid)) + } + + /// Position of the Mem column, which is clickable to sort. + pub fn mem_index(&self) -> Option { + self.mem + .then(|| 1 + usize::from(self.pid) + usize::from(self.cpu)) + } +} pub fn draw_top_processes(f: &mut ratatui::Frame<'_>, area: Rect, params: ProcessDisplayParams) { // Draw outer block and title @@ -233,6 +302,8 @@ pub fn draw_top_processes(f: &mut ratatui::Frame<'_>, area: Rect, params: Proces .fold(0.0_f32, f32::max) }; + let columns = ProcColumns::for_width(content.width); + let rows_iter = idxs.iter().skip(offset).take(show_n).map(|&ix| { let p = &mm.top_processes[ix]; @@ -304,16 +375,29 @@ pub fn draw_top_processes(f: &mut ratatui::Frame<'_>, area: Rect, params: Proces .add_modifier(Modifier::BOLD); } - ratatui::widgets::Row::new(vec![ - ratatui::widgets::Cell::from(pid_span).style(Style::default().fg(Color::DarkGray)), - ratatui::widgets::Cell::from(name_span), - ratatui::widgets::Cell::from(Span::raw(cpu_span_text)) - .style(Style::default().fg(cpu_fg)), - ratatui::widgets::Cell::from(Span::raw(mem_span_text)), - ratatui::widgets::Cell::from(Span::raw(mem_pct_span_text)) - .style(Style::default().fg(mem_fg)), - ]) - .style(emphasis) + let mut cells = Vec::with_capacity(5); + if columns.pid { + cells.push( + ratatui::widgets::Cell::from(pid_span).style(Style::default().fg(Color::DarkGray)), + ); + } + cells.push(ratatui::widgets::Cell::from(name_span)); + if columns.cpu { + cells.push( + ratatui::widgets::Cell::from(Span::raw(cpu_span_text)) + .style(Style::default().fg(cpu_fg)), + ); + } + if columns.mem { + cells.push(ratatui::widgets::Cell::from(Span::raw(mem_span_text))); + } + if columns.mem_pct { + cells.push( + ratatui::widgets::Cell::from(Span::raw(mem_pct_span_text)) + .style(Style::default().fg(mem_fg)), + ); + } + ratatui::widgets::Row::new(cells).style(emphasis) }); // Header with sort indicator @@ -325,16 +409,30 @@ pub fn draw_top_processes(f: &mut ratatui::Frame<'_>, area: Rect, params: Proces ProcSortBy::MemDesc => "Mem •", _ => "Mem", }; - let header = ratatui::widgets::Row::new(vec!["PID", "Name", cpu_hdr, mem_hdr, "Mem %"]).style( + let mut header_cells = Vec::with_capacity(5); + if columns.pid { + header_cells.push("PID"); + } + header_cells.push("Name"); + if columns.cpu { + header_cells.push(cpu_hdr); + } + if columns.mem { + header_cells.push(mem_hdr); + } + if columns.mem_pct { + header_cells.push("Mem %"); + } + let header = ratatui::widgets::Row::new(header_cells).style( Style::default() .fg(Color::Cyan) .add_modifier(Modifier::BOLD), ); // Render table inside content area (no borders here; outer block already drawn) - let table = Table::new(rows_iter, COLS.to_vec()) + let table = Table::new(rows_iter, columns.constraints()) .header(header) - .column_spacing(1); + .column_spacing(COL_SPACING); f.render_widget(table, content); // Draw tooltip if a process is selected @@ -546,15 +644,24 @@ pub fn processes_handle_mouse( && mouse.column < header_area.x + header_area.width; if inside_header && matches!(mouse.kind, MouseEventKind::Down(MouseButton::Left)) { - // Split header into the same columns + // Split the header the same way the draw path did, so a click lands on the + // column actually on screen even when PID has been dropped. + let columns = ProcColumns::for_width(header_area.width); let cols = Layout::default() .direction(Direction::Horizontal) - .constraints(COLS.to_vec()) + .constraints(columns.constraints()) + .spacing(COL_SPACING) // must match Table::column_spacing in the draw path .split(header_area); - if mouse.column >= cols[2].x && mouse.column < cols[2].x + cols[2].width { + if let Some(cpu) = columns.cpu_index().map(|i| cols[i]) + && mouse.column >= cpu.x + && mouse.column < cpu.x + cpu.width + { return Some(ProcSortBy::CpuDesc); } - if mouse.column >= cols[3].x && mouse.column < cols[3].x + cols[3].width { + if let Some(mem) = columns.mem_index().map(|i| cols[i]) + && mouse.column >= mem.x + && mouse.column < mem.x + mem.width + { return Some(ProcSortBy::MemDesc); } } @@ -646,15 +753,24 @@ pub fn processes_handle_mouse_with_selection(params: ProcessMouseParams) -> Opti && params.mouse.column < header_area.x + header_area.width; if inside_header && matches!(params.mouse.kind, MouseEventKind::Down(MouseButton::Left)) { - // Split header into the same columns + // Split the header the same way the draw path did, so a click lands on the + // column actually on screen even when PID has been dropped. + let columns = ProcColumns::for_width(header_area.width); let cols = Layout::default() .direction(Direction::Horizontal) - .constraints(COLS.to_vec()) + .constraints(columns.constraints()) + .spacing(COL_SPACING) // must match Table::column_spacing in the draw path .split(header_area); - if params.mouse.column >= cols[2].x && params.mouse.column < cols[2].x + cols[2].width { + if let Some(cpu) = columns.cpu_index().map(|i| cols[i]) + && params.mouse.column >= cpu.x + && params.mouse.column < cpu.x + cpu.width + { return Some(ProcSortBy::CpuDesc); } - if params.mouse.column >= cols[3].x && params.mouse.column < cols[3].x + cols[3].width { + if let Some(mem) = columns.mem_index().map(|i| cols[i]) + && params.mouse.column >= mem.x + && params.mouse.column < mem.x + mem.width + { return Some(ProcSortBy::MemDesc); } } @@ -691,3 +807,258 @@ pub fn processes_handle_mouse_with_selection(params: ProcessMouseParams) -> Opti ); None } + +#[cfg(test)] +mod column_tests { + use super::*; + use ratatui::layout::{Direction, Layout, Rect}; + + fn name_width(w: u16) -> u16 { + let c = ProcColumns::for_width(w); + let rects = Layout::default() + .direction(Direction::Horizontal) + .constraints(c.constraints()) + .spacing(COL_SPACING) + .split(Rect::new(0, 0, w, 1)); + rects[usize::from(c.pid)].width + } + + /// The complaint this fixes: on a narrow pane the Name column was the first thing to + /// disappear, leaving a table of numbers with nothing to identify the process. Name + /// must now be the last column standing, at every width that can render anything. + #[test] + fn name_is_never_the_column_that_gets_dropped() { + for width in NAME_MIN_W..=200u16 { + assert!( + name_width(width) >= 1, + "width {width}: Name was squeezed to nothing" + ); + } + } + + /// Columns are shed in reverse priority order, so a narrower pane can never show a + /// column that a wider one hid. + #[test] + fn columns_are_shed_in_priority_order() { + for width in 0..=200u16 { + let c = ProcColumns::for_width(width); + assert!(!c.mem_pct || c.pid, "width {width}: Mem % outlived PID"); + assert!(!c.pid || c.mem, "width {width}: PID outlived Mem"); + assert!(!c.mem || c.cpu, "width {width}: Mem outlived CPU %"); + } + } + + /// Columns come back as the pane widens and never flap. + #[test] + fn columns_are_monotonic_in_width() { + let mut prev = ProcColumns::for_width(0); + for width in 1..=200u16 { + let c = ProcColumns::for_width(width); + for (was, now, name) in [ + (prev.cpu, c.cpu, "CPU %"), + (prev.mem, c.mem, "Mem"), + (prev.pid, c.pid, "PID"), + (prev.mem_pct, c.mem_pct, "Mem %"), + ] { + assert!(!was || now, "width {width}: {name} vanished as it widened"); + } + prev = c; + } + } + + /// The tiers, from a comfortable pane down to a very narrow one. + #[test] + fn narrow_panes_shed_columns_in_order() { + let full = ProcColumns::for_width(48); + assert_eq!(full.constraints().len(), 5); + assert!(full.pid && full.cpu && full.mem && full.mem_pct); + + // Mem % goes first. + let c = ProcColumns::for_width(45); + assert!(c.pid && c.mem && !c.mem_pct); + + // Then PID. + let c = ProcColumns::for_width(35); + assert!(!c.pid && c.cpu && c.mem); + + // Then Mem, leaving the name and its CPU load. + let c = ProcColumns::for_width(20); + assert!(!c.mem && c.cpu); + assert_eq!(c.constraints().len(), 2); + + // At the floor, just the name. + let c = ProcColumns::for_width(10); + assert!(!c.cpu && !c.mem); + assert_eq!(c.constraints().len(), 1); + } + + /// Regression guard for the old behaviour: a 130-column terminal gives the process + /// pane ~48 columns, and every column still fits there. + #[test] + fn a_wide_terminal_keeps_the_full_table() { + assert_eq!(ProcColumns::for_width(48).constraints().len(), 5); + } + + /// Sort clicks are resolved by index, so those indices must track the columns that + /// are actually rendered — otherwise clicking "CPU %" would sort by Mem. + #[test] + fn sort_indices_follow_the_rendered_columns() { + let wide = ProcColumns::for_width(48); + assert_eq!(wide.cpu_index(), Some(2)); // PID, Name, CPU % + assert_eq!(wide.mem_index(), Some(3)); + + let narrow = ProcColumns::for_width(35); + assert_eq!(narrow.cpu_index(), Some(1)); // Name, CPU % + assert_eq!(narrow.mem_index(), Some(2)); + + // A column that is not rendered has no index to click. + let tiny = ProcColumns::for_width(10); + assert_eq!(tiny.cpu_index(), None); + assert_eq!(tiny.mem_index(), None); + + // Whatever the width, any index returned is inside the rendered set. + for width in 0..=200u16 { + let c = ProcColumns::for_width(width); + let n = c.constraints().len(); + for i in [c.cpu_index(), c.mem_index()].into_iter().flatten() { + assert!(i < n, "width {width}: index {i} outside {n} columns"); + } + } + } + + /// Name takes the slack, so it grows with the pane instead of being pinned to a + /// percentage that the fixed columns can crush. + #[test] + fn name_absorbs_the_leftover_width() { + assert!( + name_width(80) > name_width(60), + "Name did not grow with the pane" + ); + } +} + +#[cfg(test)] +mod click_tests { + use super::*; + use crossterm::event::{KeyModifiers, MouseButton, MouseEvent, MouseEventKind}; + use ratatui::Terminal; + use ratatui::backend::TestBackend; + use ratatui::layout::Rect; + use socktop_connector::{Metrics, ProcessInfo}; + + fn metrics() -> Metrics { + Metrics { + cpu_total: 0.0, + cpu_per_core: vec![], + mem_total: 32_000_000_000, + mem_used: 0, + swap_total: 0, + swap_used: 0, + hostname: "t".into(), + cpu_temp_c: None, + disks: vec![], + networks: vec![], + top_processes: vec![ProcessInfo { + pid: 4242, + name: "some-process".into(), + cpu_usage: 1.5, + mem_bytes: 1_000_000, + }], + gpus: None, + process_count: Some(1), + } + } + + /// Renders the pane and returns its header row as text. + fn header_row(width: u16) -> String { + let m = metrics(); + let mut cache = Vec::new(); + let peak = rebuild_row_cache(&m, &mut cache); + let idxs = [0usize]; + let mut terminal = Terminal::new(TestBackend::new(width, 8)).unwrap(); + terminal + .draw(|f| { + draw_top_processes( + f, + Rect::new(0, 0, width, 8), + ProcessDisplayParams { + metrics: Some(&m), + scroll_offset: 0, + sort_by: ProcSortBy::CpuDesc, + selected_process_pid: None, + selected_process_index: None, + search_query: "", + search_active: false, + filtered_indices: &idxs, + cached_rows: &cache, + peak_cpu: peak, + }, + ) + }) + .unwrap(); + let buf = terminal.backend().buffer(); + (0..width) + .map(|x| buf[(x, 1)].symbol().to_string()) + .collect() + } + + fn click(width: u16, column: u16) -> Option { + let mut scroll = 0usize; + let mut drag = None; + processes_handle_mouse( + &mut scroll, + &mut drag, + MouseEvent { + kind: MouseEventKind::Down(MouseButton::Left), + column, + row: 1, + modifiers: KeyModifiers::NONE, + }, + Rect::new(0, 0, width, 8), + 1, + ) + } + + /// The hit-test rects are computed by a separate `Layout` call from the one `Table` + /// renders with. This walks the rendered header text and clicks each label where it + /// actually appears, which catches any drift between the two — including column + /// spacing, which the two APIs configure differently. + #[test] + fn clicking_a_rendered_sort_header_sorts_by_that_column() { + for width in [40u16, 50, 60, 80, 120] { + let row = header_row(width); + let cpu_at = row.find("CPU").map(|i| row[..i].chars().count() as u16); + let mem_at = row.find("Mem").map(|i| row[..i].chars().count() as u16); + + if let Some(x) = cpu_at { + assert_eq!( + click(width, x), + Some(ProcSortBy::CpuDesc), + "width {width}: clicking the rendered 'CPU %' header at column {x} \ + did not sort by CPU (header row: {row:?})" + ); + } + if let Some(x) = mem_at { + assert_eq!( + click(width, x), + Some(ProcSortBy::MemDesc), + "width {width}: clicking the rendered 'Mem' header at column {x} \ + did not sort by Mem (header row: {row:?})" + ); + } + } + } + + /// Name is what identifies the row, so it must be rendered at every width the pane + /// can draw anything at. + #[test] + fn the_name_column_is_rendered_even_when_narrow() { + for width in [30u16, 40, 60, 120] { + let row = header_row(width); + assert!( + row.contains("Name"), + "width {width}: no Name column in header {row:?}" + ); + } + } +}