From 3f2f1752e749fe6b90e91172aa9f3e7022dd5c03 Mon Sep 17 00:00:00 2001 From: Leszek Date: Fri, 31 Jul 2026 15:25:43 +0200 Subject: [PATCH] =?UTF-8?q?chore(ui):=20debt=20=E2=80=94=20input,=20dialog?= =?UTF-8?q?s,=20viewer,=20render?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/input/command_line.rs | 1 - src/render.rs | 13 ++++++------- src/ui/mod.rs | 5 ----- src/ui/panels/tests.rs | 2 -- src/ui/theme.rs | 38 ++++++++++++++++++++++++++------------ src/ui/viewer/hex.rs | 2 +- src/ui/viewer/render.rs | 5 +++-- src/ui/viewer/tests.rs | 2 +- src/ui/viewer/toggle.rs | 3 +-- 9 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/input/command_line.rs b/src/input/command_line.rs index bd2d1b7..ad8ef67 100644 --- a/src/input/command_line.rs +++ b/src/input/command_line.rs @@ -96,7 +96,6 @@ pub(crate) fn handle_command_line(state: &mut AppState, key: KeyEvent) { KeyCode::Up if !state.input.command_history.is_empty() => { if state.input.history_index.is_none() { state.input.command_draft = state.input.command_line.text().to_owned(); - state.input.command_line.set_text(String::new()); } let idx = match state.input.history_index { Some(i) if i > 0 => i - 1, diff --git a/src/render.rs b/src/render.rs index ab4d0a5..5c63bd8 100644 --- a/src/render.rs +++ b/src/render.rs @@ -1,9 +1,15 @@ use crate::render_dialog_map; +use lc::{app, ui}; use ratatui::{ layout::{Constraint, Direction, Layout}, prelude::*, }; +use app::types::{ActivePanel, AppMode, AppState, PickerKind, ViewMode}; +use std::borrow::Cow; +use ui::theme::{ColorPalette, Theme}; +use ui::{dialogs, panels, viewer}; + fn safe_split_at(s: &str, mut byte_idx: usize) -> (&str, &str) { byte_idx = byte_idx.min(s.len()); while byte_idx > 0 && !s.is_char_boundary(byte_idx) { @@ -19,13 +25,6 @@ fn cursor_line(prefix: &str, text: &str, byte_pos: usize) -> String { format!("{prefix}{before}_{after}") } -use lc::{app, ui}; - -use app::types::{ActivePanel, AppMode, AppState, PickerKind, ViewMode}; -use std::borrow::Cow; -use ui::theme::{ColorPalette, Theme}; -use ui::{dialogs, panels, viewer}; - pub(crate) fn render_ui( f: &mut Frame, state: &AppState, diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 550c3a0..5363fe9 100755 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -5,11 +5,6 @@ pub mod panels; pub mod theme; pub mod viewer; -// Top-level re-exports of the cross-cutting theming API, mirroring the way -// `dialogs::mod` re-exports its public surface, so callers can reach the common -// types via `ui::Theme` rather than the full module path. -pub use theme::{ColorPalette, IconTheme, Theme}; - /// Number of rows reserved for UI elements outside the main panels. /// Accounts for: top menu bar (1), status bar (1), command line (1), /// function key bar (1), and borders (2). Used to calculate available diff --git a/src/ui/panels/tests.rs b/src/ui/panels/tests.rs index a8e9957..69e43cf 100644 --- a/src/ui/panels/tests.rs +++ b/src/ui/panels/tests.rs @@ -1,5 +1,3 @@ -#![allow(clippy::expect_used)] - use super::*; use crate::app::types::{format_size, format_time, sanitize_for_display}; use crate::ui::theme::{DEFAULT_COLORS, IconTheme}; diff --git a/src/ui/theme.rs b/src/ui/theme.rs index bb0a9e1..d4ca169 100644 --- a/src/ui/theme.rs +++ b/src/ui/theme.rs @@ -1,5 +1,4 @@ -use ratatui::style::Modifier; -use ratatui::style::{Color, Style}; +use ratatui::style::{Color, Modifier, Style}; use serde::Deserialize; use crate::app::types::FileCategory; @@ -53,11 +52,10 @@ macro_rules! define_theme_colors { pub struct ThemeConfig { $(pub $field: Option,)* pub preset: Option, - #[serde(default)] pub icon_theme: IconTheme, } - #[derive(Copy, Clone, Debug, PartialEq)] + #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct ColorPalette { $(pub $field: Color,)* icon_theme: IconTheme, @@ -220,16 +218,21 @@ fn parse_color(s: &str) -> Option { } fn parse_hex_color(hex: &str) -> Option { - if hex.len() == 6 { - let r = u8::from_str_radix(&hex[0..2], 16).ok()?; - let g = u8::from_str_radix(&hex[2..4], 16).ok()?; - let b = u8::from_str_radix(&hex[4..6], 16).ok()?; + // Operate on bytes and require a pure-ASCII digit string. Slicing `&hex[n..m]` + // on a multi-byte UTF-8 config value can split a codepoint and panic at + // runtime ("byte index is not a char boundary"). `is_ascii()` rejects any + // such input before we index; `as_bytes()` is safe once that holds. + let bytes = hex.as_bytes(); + if bytes.len() == 6 && hex.is_ascii() { + let r = u8::from_str_radix(std::str::from_utf8(&bytes[0..2]).ok()?, 16).ok()?; + let g = u8::from_str_radix(std::str::from_utf8(&bytes[2..4]).ok()?, 16).ok()?; + let b = u8::from_str_radix(std::str::from_utf8(&bytes[4..6]).ok()?, 16).ok()?; return Some(Color::Rgb(r, g, b)); } - if hex.len() == 3 { - let r = u8::from_str_radix(&hex[0..1], 16).ok()? * 17; - let g = u8::from_str_radix(&hex[1..2], 16).ok()? * 17; - let b = u8::from_str_radix(&hex[2..3], 16).ok()? * 17; + if bytes.len() == 3 && hex.is_ascii() { + let r = u8::from_str_radix(std::str::from_utf8(&bytes[0..1]).ok()?, 16).ok()? * 17; + let g = u8::from_str_radix(std::str::from_utf8(&bytes[1..2]).ok()?, 16).ok()? * 17; + let b = u8::from_str_radix(std::str::from_utf8(&bytes[2..3]).ok()?, 16).ok()? * 17; return Some(Color::Rgb(r, g, b)); } None @@ -470,6 +473,17 @@ mod tests { assert_eq!(parse_color("#12345"), None); } + #[test] + fn parse_hex_color_non_ascii_does_not_panic() { + // Regression: a 6-byte slice that is not char-boundary aligned (e.g. a + // multi-byte UTF-8 codepoint sitting where a hex digit is expected) + // must return None, not panic on `&hex[n..m]`. Reachable from user + // config via parse_color -> parse_hex_color. + assert_eq!(parse_hex_color("a\u{4e2d}cd"), None); + assert_eq!(parse_hex_color("\u{4e2d}ab"), None); + assert_eq!(parse_hex_color("\u{4e2d}"), None); + } + #[test] fn defaults_match_when_no_config() { let c = &DEFAULT_COLORS; diff --git a/src/ui/viewer/hex.rs b/src/ui/viewer/hex.rs index 55df58f..1bffc8e 100644 --- a/src/ui/viewer/hex.rs +++ b/src/ui/viewer/hex.rs @@ -90,7 +90,7 @@ pub(crate) fn format_hex_line_to_buffer(offset: usize, bytes: &[u8], buf: &mut S } pub(crate) fn find_bytes(haystack: &[u8], needle: &[u8]) -> Option { - if needle.is_empty() || needle.len() > haystack.len() { + if needle.is_empty() { return None; } if needle.len() == 1 { diff --git a/src/ui/viewer/render.rs b/src/ui/viewer/render.rs index d814cf5..b141cc4 100644 --- a/src/ui/viewer/render.rs +++ b/src/ui/viewer/render.rs @@ -451,10 +451,11 @@ fn render_viewer_status( let status_text = format!( " {mode_label} {mime_label} {size_label} {position_text}{utf8_warning}{binary_warning}{truncated_warning}", ); + let status_style = Theme::status_bar_with_colors(colors); let status_style = if has_warning { - Theme::status_bar_with_colors(colors).fg(colors.warning) + status_style.fg(colors.warning) } else { - Theme::status_bar_with_colors(colors) + status_style }; let status_paragraph = Paragraph::new(status_text).style(status_style); f.render_widget(status_paragraph, status_area); diff --git a/src/ui/viewer/tests.rs b/src/ui/viewer/tests.rs index acf082c..79a0e77 100644 --- a/src/ui/viewer/tests.rs +++ b/src/ui/viewer/tests.rs @@ -743,7 +743,7 @@ fn test_hex_mode_search() { "hex search for '01 02' should find matches in hex data section" ); assert_eq!(state.current_match, Some(0)); - assert!(state.search_matches[0].line == 0); + assert_eq!(state.search_matches[0].line, 0); } #[test] diff --git a/src/ui/viewer/toggle.rs b/src/ui/viewer/toggle.rs index 531a8e3..2c853f4 100644 --- a/src/ui/viewer/toggle.rs +++ b/src/ui/viewer/toggle.rs @@ -42,8 +42,7 @@ impl ViewerState { // Reset horizontal scroll on every toggle, both directions. Wrap-mode // rendering ignores `horizontal_offset`, but `scroll_right` keeps // growing it while wrapped; without this reset the view would jump by - // that stale offset the moment wrap is turned back off. (Pairs with the - // `mode_dispatch` `scroll_right` wrap-guard tracked in PR8.) + // that stale offset the moment wrap is turned back off. self.horizontal_offset = 0; self.invalidate_visual_cache(); }