9 Commits

Author SHA1 Message Date
dailz
10323ce814 fix(tui): add area offset to settings popup positioning (closes #31) 2026-06-11 16:49:37 +08:00
dailz
c1a931551b fix(tui): handle settings save errors and rewrite truncate_to_columns (closes #30)
- Report save failures in settings instead of silently discarding errors
- Save draft first, commit color_config only on success (prevents split-brain)
- Show error message in red on status bar, stay in Settings mode on failure
- Clear error on Esc, entering settings, or any settings edit
- Rewrite truncate_to_columns as dedicated O(prefix) truncator
- Tab: stop before tab if expansion would exceed width
- Fixes pre-existing test_truncate_to_columns_tab failure
2026-06-11 16:08:57 +08:00
dailz
dfc016c348 fix(tui): wrap color backward cycle at index 0 instead of clamping (closes #29) 2026-06-11 15:05:42 +08:00
dailz
19a3b877f9 fix(core): tighten word boundary to reject digits and underscores in level detection (closes #28) 2026-06-11 14:28:13 +08:00
dailz
5cb56dafd8 fix(core): correct tab-stop alignment and width overflow in wrap_line_chars
- Extract TAB_WIDTH constant (4) replacing magic numbers
- Calculate tab stop as TAB_WIDTH - (col % TAB_WIDTH) for proper alignment
- Split tab expansion across rows when width < TAB_WIDTH
- Update test_wrap_with_tab expected value for new behavior
- Add tests: narrow width, stop alignment, line boundary, regression

Fixes: #27
2026-06-11 13:37:14 +08:00
dailz
e99861c76d fix(tui): add MAX_WRAP_INPUT_LEN guard to prevent UI freeze on oversized lines (closes #26)
- compute_line_entry/compute_visual_height: double guard (raw + post-format)
- Skip detect_level on oversized raw input to avoid O(n) JSON parsing
- Add post-format guard for JSON lines that expand beyond 10MB
- progressive_reader: add post-format guard to compute_line_visual_height
- Add truncate_to_columns helper using existing wrap_line_chars
- Fix misleading docstring on MAX_WRAP_INPUT_LEN constant
- Add 6 regression tests covering all guard paths
2026-06-11 13:15:11 +08:00
dailz
a43ef673b0 fix(tui): rebase v_offset before VHI invalidation to prevent viewport jump (closes #25)
Ready state can have VHI=None during async rebuild (Tab toggle, resize,
file append, Loading→Ready transition). Without rebase, v_offset retains
a visual-row value that gets treated as a logical-line offset, causing
viewport jumps and cursor drift.

Changes:
- Add rebase_offset_for_invalidate() to convert v_offset from visual-row
  to logical-line + sub-row before VHI invalidation
- Call it at all 6 invalidation sites in Ready state
- Recalibrate v_offset from viewport top when VHI rebuild completes
- Clamp preserved sub_row to new line height after JSON/width changes

Regression tests: 6 new tests covering rebase conversion, Tab toggle,
VHI rebuild recalibration, sub-row clamping, and Loading→Ready transition.
2026-06-11 09:49:10 +08:00
dailz
70f930eef7 fix(tui): use updated v_offset after params_changed in ensure_viewport_cache (#24)
The loading branch of ensure_viewport_cache captured v_offset before the
params_changed block, which could reassign self.v_offset. This caused the
viewport to use a stale offset when loading + width/format changed together.

Remove the stale local variable and read self.v_offset directly, consistent
with the non-loading branch. Add regression test.
2026-06-11 08:58:10 +08:00
dailz
463c53148b fix(tui): filter KeyEventKind to prevent Release/Repeat from triggering commands (closes #23) 2026-06-10 17:34:17 +08:00
7 changed files with 1007 additions and 37 deletions

1
Cargo.lock generated
View File

@@ -2358,6 +2358,7 @@ dependencies = [
"ratatui", "ratatui",
"serde_json", "serde_json",
"tempfile", "tempfile",
"unicode-width",
] ]
[[package]] [[package]]

View File

@@ -223,6 +223,9 @@ pub fn compute_line_visual_height(
} }
if json_format { if json_format {
let formatted = format_json_line(line_text); let formatted = format_json_line(line_text);
if formatted.len() > MAX_WRAP_INPUT_LEN {
return 1;
}
compute_text_visual_height(&formatted, terminal_width) compute_text_visual_height(&formatted, terminal_width)
} else { } else {
compute_text_visual_height(line_text, terminal_width) compute_text_visual_height(line_text, terminal_width)

View File

@@ -1,10 +1,16 @@
/// Maximum input length for wrap/format operations (10 MB). /// Maximum input length for wrap/format operations (10 MB).
/// Lines exceeding this are returned as-is to avoid pathological cases. /// Callers should check against this constant before invoking `wrap_line_chars`
/// to avoid pathological cases on oversized lines.
pub const MAX_WRAP_INPUT_LEN: usize = 10 * 1024 * 1024; pub const MAX_WRAP_INPUT_LEN: usize = 10 * 1024 * 1024;
/// Column spacing for tab stop alignment.
const TAB_WIDTH: usize = 4;
/// Split a line into chunks of exactly `width` display columns. /// Split a line into chunks of exactly `width` display columns.
/// For a log viewer, we want character-level wrapping, not word-level. /// For a log viewer, we want character-level wrapping, not word-level.
/// Uses `unicode-width` for correct CJK/emoji/zero-width handling. /// Uses `unicode-width` for correct CJK/emoji/zero-width handling.
/// Tab characters expand to the next tab-stop boundary and split across
/// rows when the expansion exceeds the remaining width.
pub fn wrap_line_chars(line: &str, width: usize) -> Vec<String> { pub fn wrap_line_chars(line: &str, width: usize) -> Vec<String> {
use unicode_width::UnicodeWidthChar; use unicode_width::UnicodeWidthChar;
@@ -18,9 +24,24 @@ pub fn wrap_line_chars(line: &str, width: usize) -> Vec<String> {
let mut row = String::new(); let mut row = String::new();
let mut col = 0; let mut col = 0;
for ch in line.chars() { for ch in line.chars() {
let w = if ch == '\t' { if ch == '\t' {
4 let tab_stop = TAB_WIDTH - (col % TAB_WIDTH);
} else if ch.is_control() { let mut remaining = tab_stop;
while remaining > 0 {
let avail = width.saturating_sub(col);
let fill = remaining.min(avail);
for _ in 0..fill {
row.push(' ');
}
col += fill;
remaining -= fill;
if col >= width {
result.push(std::mem::take(&mut row));
col = 0;
}
}
} else {
let w = if ch.is_control() {
// Control characters (except tab): width 0, still pushed to preserve content. // Control characters (except tab): width 0, still pushed to preserve content.
// Visible rendering is the caller's responsibility. // Visible rendering is the caller's responsibility.
0 0
@@ -31,18 +52,14 @@ pub fn wrap_line_chars(line: &str, width: usize) -> Vec<String> {
result.push(std::mem::take(&mut row)); result.push(std::mem::take(&mut row));
col = 0; col = 0;
} }
if ch == '\t' {
row.push_str(" ");
col += 4;
} else {
row.push(ch); row.push(ch);
col += w; col += w;
}
if col >= width { if col >= width {
result.push(std::mem::take(&mut row)); result.push(std::mem::take(&mut row));
col = 0; col = 0;
} }
} }
}
if !row.is_empty() { if !row.is_empty() {
result.push(row); result.push(row);
} }
@@ -107,7 +124,7 @@ mod tests {
#[test] #[test]
fn test_wrap_with_tab() { fn test_wrap_with_tab() {
let result = wrap_line_chars("a\tb", 4); let result = wrap_line_chars("a\tb", 4);
assert_eq!(result, vec!["a", " ", "b"]); assert_eq!(result, vec!["a ", "b"]);
} }
#[test] #[test]
@@ -187,4 +204,31 @@ mod tests {
let result = wrap_line_chars("你好", 1); let result = wrap_line_chars("你好", 1);
assert_eq!(result, vec!["", ""]); assert_eq!(result, vec!["", ""]);
} }
#[test]
fn test_tab_narrow_width() {
let result = wrap_line_chars("\t", 2);
assert_eq!(result, vec![" ", " "]);
let result = wrap_line_chars("\t", 1);
assert_eq!(result, vec![" ", " ", " ", " "]);
}
#[test]
fn test_tab_stop_alignment() {
assert_eq!(wrap_line_chars("a\tb", 8), vec!["a b"]);
assert_eq!(wrap_line_chars("ab\t", 4), vec!["ab "]);
assert_eq!(wrap_line_chars("abc\tb", 8), vec!["abc b"]);
}
#[test]
fn test_tab_at_line_boundary() {
let result = wrap_line_chars("a\tb", 4);
assert_eq!(result, vec!["a ", "b"]);
}
#[test]
fn test_tab_regression_ab_tab() {
let result = wrap_line_chars("ab\t", 4);
assert_eq!(result, vec!["ab "]);
}
} }

View File

@@ -100,13 +100,21 @@ fn detect_level_from_text(line: &str) -> Option<LogLevel> {
) )
} }
// ─── is_ident_char ─────────────────────────────────────────────────────────
/// Whether a byte looks like an ASCII identifier continuation character
/// (letter / digit / underscore). Log-level keywords must NOT be adjacent to
/// such characters to count as a valid word boundary.
fn is_ident_char(b: u8) -> bool {
b.is_ascii_alphanumeric() || b == b'_'
}
// ─── is_word_boundary ─────────────────────────────────────────────────────── // ─── is_word_boundary ───────────────────────────────────────────────────────
/// Check that the match at `start..start+len` is surrounded by non-alphabetic /// Check that the match at `start..start+len` is surrounded by non-identifier
/// characters (or the string edge). /// characters (or the string edge).
fn is_word_boundary(text: &str, start: usize, len: usize) -> bool { fn is_word_boundary(text: &str, start: usize, len: usize) -> bool {
let before_ok = start == 0 || !text.as_bytes()[start - 1].is_ascii_alphabetic(); let before_ok = start == 0 || !is_ident_char(text.as_bytes()[start - 1]);
let after_idx = start + len; let after_idx = start + len;
let after_ok = after_idx >= text.len() || !text.as_bytes()[after_idx].is_ascii_alphabetic(); let after_ok = after_idx >= text.len() || !is_ident_char(text.as_bytes()[after_idx]);
before_ok && after_ok before_ok && after_ok
} }
@@ -209,4 +217,36 @@ mod tests {
let line = format!("{prefix} ERROR something"); let line = format!("{prefix} ERROR something");
assert_eq!(detect_level(&line), Some(LogLevel::Error)); assert_eq!(detect_level(&line), Some(LogLevel::Error));
} }
#[test]
fn test_boundary_rejects_trailing_digits() {
assert_eq!(detect_level("ERROR123"), None);
assert_eq!(detect_level("WARN2: bad"), None);
assert_eq!(detect_level("ERR2"), None);
}
#[test]
fn test_boundary_rejects_underscore() {
assert_eq!(detect_level("INFO_foo"), None);
assert_eq!(detect_level("DBG_value=5"), None);
}
#[test]
fn test_boundary_rejects_leading_digits_and_underscore() {
assert_eq!(detect_level("123ERROR: fail"), None);
assert_eq!(detect_level("foo_ERROR: fail"), None);
assert_eq!(detect_level("1WRN"), None);
}
#[test]
fn test_boundary_accepts_valid_suffixes() {
assert_eq!(detect_level("ERROR: fail"), Some(LogLevel::Error));
assert_eq!(detect_level("[ERROR] fail"), Some(LogLevel::Error));
assert_eq!(detect_level("ERROR fail"), Some(LogLevel::Error));
}
#[test]
fn test_boundary_camel_case_regression() {
assert_eq!(detect_level("errorLevel"), None);
}
} }

View File

@@ -11,6 +11,7 @@ anyhow.workspace = true
log-viewer-core.workspace = true log-viewer-core.workspace = true
serde_json.workspace = true serde_json.workspace = true
crossbeam-channel.workspace = true crossbeam-channel.workspace = true
unicode-width.workspace = true
[dev-dependencies] [dev-dependencies]
tempfile = { workspace = true } tempfile = { workspace = true }

File diff suppressed because it is too large Load Diff

View File

@@ -88,9 +88,21 @@ pub fn render(frame: &mut ratatui::Frame, app: &mut App) {
} }
// ── Status bar ───────────────────────────────────────────────── // ── Status bar ─────────────────────────────────────────────────
let status_text = if app.mode == AppMode::Settings { if app.mode == AppMode::Settings {
" j/k:navigate ←/→:change 1-8:jump Enter:save Esc:cancel" if let Some(ref err) = app.settings_error {
} else if app.is_error() { frame.render_widget(
Paragraph::new(err.as_str()).style(Style::default().fg(Color::Red)),
outer[2],
);
} else {
frame.render_widget(
Paragraph::new(" j/k:navigate ←/→:change 1-8:jump Enter:save Esc:cancel"),
outer[2],
);
}
return;
}
let status_text = if app.is_error() {
" Press q to quit" " Press q to quit"
} else if app.is_loading() { } else if app.is_loading() {
let pct = app.loading_progress().map_or(0, |p| p as usize); let pct = app.loading_progress().map_or(0, |p| p as usize);
@@ -130,8 +142,8 @@ pub fn render_settings(frame: &mut ratatui::Frame, app: &mut App, area: ratatui:
let popup_w = ((area.width as u32 * 4 / 5).max(40)).min(area.width as u32) as u16; let popup_w = ((area.width as u32 * 4 / 5).max(40)).min(area.width as u32) as u16;
let popup_h = ((area.height as u32 * 4 / 5).max(14)).min(area.height as u32) as u16; let popup_h = ((area.height as u32 * 4 / 5).max(14)).min(area.height as u32) as u16;
let popup_x = area.width.saturating_sub(popup_w) / 2; let popup_x = area.x.saturating_add(area.width.saturating_sub(popup_w) / 2);
let popup_y = area.height.saturating_sub(popup_h) / 2; let popup_y = area.y.saturating_add(area.height.saturating_sub(popup_h) / 2);
let popup = ratatui::layout::Rect::new(popup_x, popup_y, popup_w, popup_h); let popup = ratatui::layout::Rect::new(popup_x, popup_y, popup_w, popup_h);
let block = Block::new().borders(Borders::ALL).title(" Color Settings "); let block = Block::new().borders(Borders::ALL).title(" Color Settings ");
@@ -493,4 +505,74 @@ mod tests {
let _ = std::fs::remove_file(&path); let _ = std::fs::remove_file(&path);
assert!(result.is_ok()); assert!(result.is_ok());
} }
// ── Issue #31: Settings popup area offset tests ────────────────
/// Helper: enter settings mode and render to buffer.
fn render_settings_to_buffer(app: &mut App, width: u16, height: u16) -> ratatui::buffer::Buffer {
app.mode = crate::app::AppMode::Settings;
app.settings_draft = app.color_config.clone();
render_to_buffer(app, width, height)
}
/// Find the top-left corner of the popup border by scanning for '┌'.
fn find_popup_top_left(buf: &ratatui::buffer::Buffer, width: u16, height: u16) -> Option<(u16, u16)> {
for row in 0..height {
for col in 0..width {
if buf.cell((col, row)).unwrap().symbol() == "" {
return Some((col, row));
}
}
}
None
}
#[test]
fn test_settings_popup_includes_area_offset() {
// In an 80x24 frame with Layout [Length(1), Min(1), Length(1)]:
// outer[0] = title bar -> y=0
// outer[1] = content -> y=1, height=22
// outer[2] = status bar -> y=23
// The popup is centered within outer[1], so its y must be >= outer[1].y (which is 1).
let mut app = App::new();
let buf = render_settings_to_buffer(&mut app, 80, 24);
let (_px, py) = find_popup_top_left(&buf, 80, 24)
.expect("popup border '┌' should be rendered");
// outer[1].y == 1; the popup is centered inside a 22-row area,
// so popup_y must be at least 1 (not 0).
assert!(
py >= 1,
"popup top row should account for area.y offset, got y={py} (expected >= 1)"
);
}
#[test]
fn test_settings_popup_horizontal_centering_uses_area_x() {
// outer[1].x is 0 for this layout, so this mainly verifies the popup
// is centered and not shifted left. A non-zero area.x layout would
// need a different layout to trigger, but the formula is the same.
let mut app = App::new();
let buf = render_settings_to_buffer(&mut app, 80, 24);
let (px, _py) = find_popup_top_left(&buf, 80, 24)
.expect("popup border '┌' should be rendered");
// popup_w = 80*4/5 = 64, centered: (80-64)/2 = 8
assert_eq!(
px, 8,
"popup should start at x=8 (centered 64-wide popup in 80-col area)"
);
}
#[test]
fn test_settings_popup_small_frame_no_panic() {
// Frame smaller than the min popup size (40x14) should not panic.
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
let mut app = App::new();
let _buf = render_settings_to_buffer(&mut app, 30, 10);
}));
assert!(result.is_ok(), "rendering settings in a small frame should not panic");
}
} }