8 Commits

Author SHA1 Message Date
dailz
b58d66f2aa fix(io): use unicode-width for correct CJK/emoji/zero-width display width (closes #12) 2026-06-07 12:50:17 +08:00
dailz
d4679a7543 fix(io): update visual height of last line on append without trailing newline (closes #11)
When a file does not end with a newline, appending content extends the
last logical line's text and thus its visual height. The incremental
extend path in handle_file_appended only computed heights for newly
created logical lines, missing the old last line whose content changed.

Add VisualHeightIndex::replace_last_line_height() — an O(1) method that
rewrites the final prefix sum entry and total. Called before
extend_from_heights so the correct line is targeted.

Changes:
- progressive_reader.rs: add replace_last_line_height, pub with_params,
  7 VHI unit tests
- app.rs: save old_reader_line_count before update, recompute last old
  line height in extend path, 2 integration regression tests
2026-06-07 09:46:24 +08:00
dailz
8844e58cb4 Merge fix/m20-append-lines-error-handling: fix append_lines I/O error swallowing (closes #45) + clippy cleanup 2026-06-07 09:17:41 +08:00
dailz
6a2f8ecb66 fix(bench): resolve pre-existing clippy warnings in report.rs and mmap_reader.rs 2026-06-07 09:15:34 +08:00
dailz
f6081b9fe9 fix(bench): propagate I/O errors in append_lines instead of silently defaulting to 0 (closes #45) 2026-06-07 09:13:37 +08:00
dailz
97a2c6a925 fix(bench): regenerate growable file each iteration in truncate safety benchmarks (closes #44) 2026-06-07 09:02:19 +08:00
dailz
e6e0e2cc90 fix(bench): correct lines_read to actual successful reads in bench_scroll_rss
lines_read was incorrectly set to max_lines.min(total) (the loop upper bound)
instead of the actual number of successfully read lines. Now tracks lines_read
via get_line(i).is_some() counter and uses max_lines.min(total) as the correct
loop upper bound to handle empty file edge case.

Fixes #43
2026-06-07 08:50:20 +08:00
dailz
ffaf462bae Merge fix/m23-single-frame-tail-overlap: [M23] small file single_frame_tail overlap fix 2026-06-07 08:32:58 +08:00
11 changed files with 290 additions and 29 deletions

1
Cargo.lock generated
View File

@@ -2333,6 +2333,7 @@ dependencies = [
"tempfile", "tempfile",
"thiserror 2.0.18", "thiserror 2.0.18",
"toml", "toml",
"unicode-width",
"xxhash-rust", "xxhash-rust",
] ]

View File

@@ -27,3 +27,4 @@ textwrap = "0.16"
tempfile = "3" tempfile = "3"
xxhash-rust = { version = "0.8", features = ["xxh3"] } xxhash-rust = { version = "0.8", features = ["xxh3"] }
bincode = "1" bincode = "1"
unicode-width = "0.2"

View File

@@ -83,11 +83,11 @@ pub fn generate_growable_file(dir: &Path) -> std::io::Result<PathBuf> {
/// Append `count` lines to the file /// Append `count` lines to the file
pub fn append_lines(path: &Path, count: usize) -> std::io::Result<()> { pub fn append_lines(path: &Path, count: usize) -> std::io::Result<()> {
let existing_lines = count_existing_lines(path)?;
let mut file = BufWriter::with_capacity( let mut file = BufWriter::with_capacity(
64 * 1024, 64 * 1024,
fs::OpenOptions::new().append(true).open(path)?, fs::OpenOptions::new().append(true).open(path)?,
); );
let existing_lines = count_existing_lines(path).unwrap_or(0);
for i in 0..count { for i in 0..count {
writeln!( writeln!(
file, file,
@@ -117,7 +117,12 @@ pub fn rotate_file(path: &Path) -> std::io::Result<PathBuf> {
fn count_existing_lines(path: &Path) -> std::io::Result<u64> { fn count_existing_lines(path: &Path) -> std::io::Result<u64> {
let file = fs::File::open(path)?; let file = fs::File::open(path)?;
let reader = BufReader::new(file); let reader = BufReader::new(file);
Ok(reader.lines().count() as u64) let mut count = 0u64;
for line in reader.lines() {
line?;
count += 1;
}
Ok(count)
} }
#[cfg(test)] #[cfg(test)]

View File

@@ -35,7 +35,7 @@ const HANDLER_NONE: u8 = 0;
const HANDLER_DEFAULT: u8 = 1; const HANDLER_DEFAULT: u8 = 1;
const HANDLER_IGNORE: u8 = 2; const HANDLER_IGNORE: u8 = 2;
const HANDLER_PLAIN: u8 = 3; // extern "C" fn(c_int) const HANDLER_PLAIN: u8 = 3; // extern "C" fn(c_int)
#[expect(clippy::unseparated_literal_suffix, reason = "clarity: this is the SA_SIGACTION variant")] #[allow(clippy::unseparated_literal_suffix, reason = "clarity: this is the SA_SIGACTION variant")]
const HANDLER_SIGACTION: u8 = 4; // extern "C" fn(c_int, *mut siginfo_t, *mut c_void) const HANDLER_SIGACTION: u8 = 4; // extern "C" fn(c_int, *mut siginfo_t, *mut c_void)
/// Old SIGBUS handler type — raw atomic, async-signal-safe to read. /// Old SIGBUS handler type — raw atomic, async-signal-safe to read.

View File

@@ -143,8 +143,7 @@ pub fn format_report(results: &[BenchmarkResult]) -> String {
report.push_str("| Test | Variant | RSS | Peak RSS | Page Faults |\n"); report.push_str("| Test | Variant | RSS | Peak RSS | Page Faults |\n");
report.push_str("|------|---------|-----|----------|-------------|\n"); report.push_str("|------|---------|-----|----------|-------------|\n");
let mut mem_rows: Vec<&BenchmarkResult> = let mut mem_rows: Vec<&BenchmarkResult> = category_results.to_vec();
category_results.iter().copied().collect();
mem_rows.sort_by(|a, b| { mem_rows.sort_by(|a, b| {
(&a.test_name, &a.backend, &a.variant) (&a.test_name, &a.backend, &a.variant)
.cmp(&(&b.test_name, &b.backend, &b.variant)) .cmp(&(&b.test_name, &b.backend, &b.variant))
@@ -163,7 +162,10 @@ pub fn format_report(results: &[BenchmarkResult]) -> String {
report.push('\n'); report.push('\n');
} }
let mut extras: Vec<(String, String, Vec<(String, f64)>)> = category_results type ExtraEntry = (String, f64);
type ExtraGroup = (String, String, Vec<ExtraEntry>);
let mut extras: Vec<ExtraGroup> = category_results
.iter() .iter()
.filter(|r| !r.extra.is_empty()) .filter(|r| !r.extra.is_empty())
.map(|r| { .map(|r| {

View File

@@ -101,11 +101,15 @@ fn bench_scroll_rss<B: FileReaderBackend>(
let sample_interval = 100_000; let sample_interval = 100_000;
let max_lines = if config.quick_mode { 100_000 } else { total }; let max_lines = if config.quick_mode { 100_000 } else { total };
let upper = max_lines.min(total);
let mut rss_samples = Vec::new(); let mut rss_samples = Vec::new();
let mut hwm_samples = Vec::new(); let mut hwm_samples = Vec::new();
let mut lines_read = 0usize;
for i in (0..max_lines).step_by(sample_interval) { for i in (0..upper).step_by(sample_interval) {
let _ = reader.get_line(i); if reader.get_line(i).is_some() {
lines_read += 1;
}
let rss = MetricsCollector::read_rss(); let rss = MetricsCollector::read_rss();
rss_samples.push(rss.vm_rss_kb); rss_samples.push(rss.vm_rss_kb);
hwm_samples.push(rss.vm_hwm_kb); hwm_samples.push(rss.vm_hwm_kb);
@@ -124,7 +128,7 @@ fn bench_scroll_rss<B: FileReaderBackend>(
"max_hwm_kb".into(), "max_hwm_kb".into(),
hwm_samples.iter().copied().fold(0u64, u64::max) as f64, hwm_samples.iter().copied().fold(0u64, u64::max) as f64,
); );
extra.insert("lines_read".into(), max_lines.min(total) as f64); extra.insert("lines_read".into(), lines_read as f64);
reader.close(); reader.close();

View File

@@ -24,13 +24,14 @@ fn bench_truncate_safety_mmap(
dir: &std::path::Path, dir: &std::path::Path,
) -> Vec<BenchmarkResult> { ) -> Vec<BenchmarkResult> {
let sub_dir = dir.join("trunc_mmap"); let sub_dir = dir.join("trunc_mmap");
let path = data_gen::generate_growable_file(&sub_dir).expect("Failed to create file");
let iterations: usize = if _config.quick_mode { 3 } else { 10 }; let iterations: usize = if _config.quick_mode { 3 } else { 10 };
let mut latencies = Vec::with_capacity(iterations); let mut latencies = Vec::with_capacity(iterations);
let mut sigbus_detected = 0usize; let mut sigbus_detected = 0usize;
for _ in 0..iterations { for _ in 0..iterations {
let path = data_gen::generate_growable_file(&sub_dir).expect("Failed to create file");
mmap_reader::reset_sigbus_flag(); mmap_reader::reset_sigbus_flag();
let reader = MmapReaderPlain::open(&path).expect("Failed to open file"); let reader = MmapReaderPlain::open(&path).expect("Failed to open file");
@@ -48,12 +49,6 @@ fn bench_truncate_safety_mmap(
sigbus_detected += 1; sigbus_detected += 1;
} }
reader.close(); reader.close();
let mut f = std::fs::File::create(&path).expect("Failed to recreate file");
use std::io::Write;
for i in 0..1000u64 {
writeln!(f, "restored line {i}").unwrap();
}
} }
let rss = MetricsCollector::read_rss(); let rss = MetricsCollector::read_rss();
@@ -82,13 +77,14 @@ fn bench_truncate_safety_pread(
dir: &std::path::Path, dir: &std::path::Path,
) -> Vec<BenchmarkResult> { ) -> Vec<BenchmarkResult> {
let sub_dir = dir.join("trunc_pread"); let sub_dir = dir.join("trunc_pread");
let path = data_gen::generate_growable_file(&sub_dir).expect("Failed to create file");
let iterations: usize = if _config.quick_mode { 3 } else { 10 }; let iterations: usize = if _config.quick_mode { 3 } else { 10 };
let mut latencies = Vec::with_capacity(iterations); let mut latencies = Vec::with_capacity(iterations);
let mut error_count = 0usize; let mut error_count = 0usize;
for _ in 0..iterations { for _ in 0..iterations {
let path = data_gen::generate_growable_file(&sub_dir).expect("Failed to create file");
let reader = PreadReaderPlain::open(&path).expect("Failed to open file"); let reader = PreadReaderPlain::open(&path).expect("Failed to open file");
let original_size = reader.file_size(); let original_size = reader.file_size();
@@ -104,12 +100,6 @@ fn bench_truncate_safety_pread(
error_count += 1; error_count += 1;
} }
reader.close(); reader.close();
let mut f = std::fs::File::create(&path).expect("Failed to recreate file");
use std::io::Write;
for i in 0..1000u64 {
writeln!(f, "restored line {i}").unwrap();
}
} }
let rss = MetricsCollector::read_rss(); let rss = MetricsCollector::read_rss();

View File

@@ -17,6 +17,7 @@ memmap2.workspace = true
directories.workspace = true directories.workspace = true
xxhash-rust.workspace = true xxhash-rust.workspace = true
bincode.workspace = true bincode.workspace = true
unicode-width.workspace = true
[dev-dependencies] [dev-dependencies]
insta.workspace = true insta.workspace = true

View File

@@ -102,7 +102,7 @@ impl VisualHeightIndex {
} }
} }
fn with_params(mut self, json_format: bool, terminal_width: usize) -> Self { pub fn with_params(mut self, json_format: bool, terminal_width: usize) -> Self {
self.json_format = json_format; self.json_format = json_format;
self.terminal_width = terminal_width; self.terminal_width = terminal_width;
self self
@@ -159,6 +159,31 @@ impl VisualHeightIndex {
self.total_visual_rows += h as u64; self.total_visual_rows += h as u64;
} }
} }
/// Replace the visual height of the last logical line. O(1).
///
/// Must be called **before** `extend_from_heights` so that the last line
/// index still refers to the pre-extension line.
pub fn replace_last_line_height(&mut self, new_height: usize) {
let n = self.prefix_sums.len();
if n < 2 {
return;
}
let last_line = n - 2;
let old_height = self.prefix_sums[last_line + 1] - self.prefix_sums[last_line];
let new_height = new_height as u64;
if new_height == old_height {
return;
}
let delta = new_height.abs_diff(old_height);
if new_height > old_height {
self.prefix_sums[last_line + 1] += delta;
self.total_visual_rows += delta;
} else {
self.prefix_sums[last_line + 1] -= delta;
self.total_visual_rows -= delta;
}
}
} }
// ─── VisualHeightRebuildResult ──────────────────────────────────────────────── // ─── VisualHeightRebuildResult ────────────────────────────────────────────────
@@ -1339,6 +1364,85 @@ mod tests {
assert_eq!(idx.total_visual_rows(), 6); assert_eq!(idx.total_visual_rows(), 6);
} }
#[test]
fn test_replace_last_line_height_increase() {
let heights = [2, 3];
let mut idx = VisualHeightIndex::build(&heights);
assert_eq!(idx.total_visual_rows(), 5);
assert_eq!(idx.visual_height_of_line(1), 3);
idx.replace_last_line_height(7);
assert_eq!(idx.visual_height_of_line(0), 2);
assert_eq!(idx.visual_height_of_line(1), 7);
assert_eq!(idx.total_visual_rows(), 9);
assert_eq!(idx.cursor_to_first_visual_row(0), 0);
assert_eq!(idx.cursor_to_first_visual_row(1), 2);
}
#[test]
fn test_replace_last_line_height_decrease() {
let heights = [2, 5];
let mut idx = VisualHeightIndex::build(&heights);
idx.replace_last_line_height(1);
assert_eq!(idx.visual_height_of_line(0), 2);
assert_eq!(idx.visual_height_of_line(1), 1);
assert_eq!(idx.total_visual_rows(), 3);
}
#[test]
fn test_replace_last_line_height_same_is_noop() {
let heights = [2, 3];
let mut idx = VisualHeightIndex::build(&heights);
let total_before = idx.total_visual_rows();
idx.replace_last_line_height(3);
assert_eq!(idx.total_visual_rows(), total_before);
}
#[test]
fn test_replace_last_line_height_then_extend() {
let heights = [2, 1];
let mut idx = VisualHeightIndex::build(&heights);
assert_eq!(idx.total_visual_rows(), 3);
idx.replace_last_line_height(4);
idx.extend_from_heights(&[3]);
assert_eq!(idx.line_count(), 3);
assert_eq!(idx.visual_height_of_line(0), 2);
assert_eq!(idx.visual_height_of_line(1), 4);
assert_eq!(idx.visual_height_of_line(2), 3);
assert_eq!(idx.total_visual_rows(), 9);
assert_eq!(idx.cursor_to_first_visual_row(0), 0);
assert_eq!(idx.cursor_to_first_visual_row(1), 2);
assert_eq!(idx.cursor_to_first_visual_row(2), 6);
}
#[test]
fn test_replace_last_line_height_single_line() {
let heights = [5];
let mut idx = VisualHeightIndex::build(&heights);
idx.replace_last_line_height(2);
assert_eq!(idx.visual_height_of_line(0), 2);
assert_eq!(idx.total_visual_rows(), 2);
}
#[test]
fn test_replace_last_line_height_empty_index() {
let heights: [usize; 0] = [];
let mut idx = VisualHeightIndex::build(&heights);
idx.replace_last_line_height(5);
assert_eq!(idx.total_visual_rows(), 0);
}
#[test] #[test]
fn test_spawn_indexer_file_truncated_during_scan() { fn test_spawn_indexer_file_truncated_during_scan() {
let mut content = Vec::new(); let mut content = Vec::new();

View File

@@ -2,9 +2,12 @@
/// Lines exceeding this are returned as-is to avoid pathological cases. /// Lines exceeding this are returned as-is to avoid pathological cases.
pub const MAX_WRAP_INPUT_LEN: usize = 10 * 1024 * 1024; pub const MAX_WRAP_INPUT_LEN: usize = 10 * 1024 * 1024;
/// Split a line into chunks of exactly `width` characters (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.
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;
if width == 0 { if width == 0 {
return vec![String::new()]; return vec![String::new()];
} }
@@ -15,7 +18,15 @@ 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' { 4 } else { 1 }; let w = if ch == '\t' {
4
} else if ch.is_control() {
// Control characters (except tab): width 0, still pushed to preserve content.
// Visible rendering is the caller's responsibility.
0
} else {
ch.width().unwrap_or(0)
};
if col + w > width && !row.is_empty() { if col + w > width && !row.is_empty() {
result.push(std::mem::take(&mut row)); result.push(std::mem::take(&mut row));
col = 0; col = 0;
@@ -132,4 +143,48 @@ mod tests {
fn test_max_wrap_input_len_constant() { fn test_max_wrap_input_len_constant() {
assert_eq!(MAX_WRAP_INPUT_LEN, 10 * 1024 * 1024); assert_eq!(MAX_WRAP_INPUT_LEN, 10 * 1024 * 1024);
} }
#[test]
fn test_wrap_cjk_chars() {
let result = wrap_line_chars("你好", 3);
assert_eq!(result, vec!["", ""]);
}
#[test]
fn test_wrap_cjk_ascii_mixed() {
let result = wrap_line_chars("a你好", 4);
assert_eq!(result, vec!["a你", ""]);
}
#[test]
fn test_wrap_zero_width_char() {
let result = wrap_line_chars("a\u{200B}b", 2);
assert_eq!(result, vec!["a\u{200B}b"]);
}
#[test]
fn test_wrap_emoji() {
let result = wrap_line_chars("😀a", 3);
assert_eq!(result, vec!["😀a"]);
}
#[test]
fn test_wrap_emoji_exact_wrap() {
let result = wrap_line_chars("😀a", 2);
assert_eq!(result, vec!["😀", "a"]);
}
#[test]
fn test_wrap_combining_mark() {
// Scalar-width wrapping: combining mark (width 0) stays with next base char,
// not the preceding one, because the base char already triggered a flush.
let result = wrap_line_chars("a\u{0301}b", 1);
assert_eq!(result, vec!["a", "\u{0301}b"]);
}
#[test]
fn test_wrap_cjk_width_one() {
let result = wrap_line_chars("你好", 1);
assert_eq!(result, vec!["", ""]);
}
} }

View File

@@ -865,6 +865,7 @@ impl App {
let width = self.get_content_width(); let width = self.get_content_width();
match &mut self.loading_state { match &mut self.loading_state {
AppLoadingState::Ready { reader } => { AppLoadingState::Ready { reader } => {
let old_reader_line_count = reader.line_count();
let status = reader.update_for_append(); let status = reader.update_for_append();
match status { match status {
Ok( Ok(
@@ -883,13 +884,24 @@ impl App {
}; };
let new_line_count = reader.line_count(); let new_line_count = reader.line_count();
if can_extend && new_line_count > old_line_count { if can_extend && old_line_count == old_reader_line_count {
if let log_viewer_core::io::progressive_reader::ReaderState::Ready { if let log_viewer_core::io::progressive_reader::ReaderState::Ready {
visual_height_index: Some(index), visual_height_index: Some(index),
reader: fr, reader: fr,
} = &mut reader.state } = &mut reader.state
{ {
let mut new_heights = Vec::with_capacity(new_line_count - old_line_count); if old_line_count > 0 {
let last_old_line_text =
fr.get_line(old_line_count - 1).unwrap_or("");
let new_h = compute_line_visual_height(
last_old_line_text,
width,
self.json_format,
);
index.replace_last_line_height(new_h);
}
let mut new_heights =
Vec::with_capacity(new_line_count.saturating_sub(old_line_count));
for i in old_line_count..new_line_count { for i in old_line_count..new_line_count {
let line_text = fr.get_line(i).unwrap_or(""); let line_text = fr.get_line(i).unwrap_or("");
new_heights.push(compute_line_visual_height( new_heights.push(compute_line_visual_height(
@@ -2473,7 +2485,9 @@ plain text line
} }
fn install_vhi(app: &mut App, heights: &[usize]) { fn install_vhi(app: &mut App, heights: &[usize]) {
let vhi = VisualHeightIndex::build(heights); let width = app.get_content_width();
let json_format = app.json_format;
let vhi = VisualHeightIndex::build(heights).with_params(json_format, width);
if let AppLoadingState::Ready { reader } = &mut app.loading_state { if let AppLoadingState::Ready { reader } = &mut app.loading_state {
if let log_viewer_core::io::progressive_reader::ReaderState::Ready { if let log_viewer_core::io::progressive_reader::ReaderState::Ready {
visual_height_index, visual_height_index,
@@ -2800,4 +2814,88 @@ plain text line
cleanup(&path); cleanup(&path);
assert!(result.is_ok()); assert!(result.is_ok());
} }
#[test]
fn test_append_no_trailing_newline_updates_last_line_height() {
let path = make_temp_file("abc");
let result = std::panic::catch_unwind(|| {
let mut app = App::new();
load_file_ready(&mut app, &path);
assert_eq!(app.total_lines(), 1);
app.content_width = 5;
install_vhi(&mut app, &[1usize]);
{
let vhi = app.get_visual_height_index().unwrap();
assert_eq!(vhi.visual_height_of_line(0), 1);
assert_eq!(vhi.total_visual_rows(), 1);
}
// Append content that extends line 0 (no trailing newline before)
// "abc" + "defgh\n" = "abcdefgh\n" → 8 chars in width 5 → wraps to 2 visual rows
// old_total=1, old_had_trailing=false → starts_new_line=false
// new_has_trailing=true, new_newlines=1 → added = 1-1 = 0
// Still 1 logical line, but line 0 text changed from "abc" to "abcdefgh"
{
use std::io::Write;
let mut f = std::fs::OpenOptions::new()
.append(true)
.open(&path)
.unwrap();
f.write_all(b"defgh\n").unwrap();
}
std::thread::sleep(std::time::Duration::from_millis(500));
app.poll_file_watcher();
assert_eq!(app.total_lines(), 1,
"\"abcdefgh\\n\" has trailing newline → 1 logical line");
let vhi = app.get_visual_height_index().expect("VHI should still exist after append");
assert_eq!(vhi.visual_height_of_line(0), 2,
"line 0 height should be updated from 1 to 2 after extending 'abcdefgh' in width 5");
assert_eq!(vhi.total_visual_rows(), 2);
assert_eq!(vhi.cursor_to_first_visual_row(0), 0);
cleanup(&path);
});
assert!(result.is_ok());
}
#[test]
fn test_append_no_trailing_newline_no_new_lines_only_height_change() {
let path = make_temp_file("abc");
let result = std::panic::catch_unwind(|| {
let mut app = App::new();
load_file_ready(&mut app, &path);
assert_eq!(app.total_lines(), 1);
app.content_width = 5;
install_vhi(&mut app, &[1usize]);
// Append without adding any new line — just extends line 0
// "abc" + "def" = "abcdef" → 6 chars in width 5 → wraps to 2 visual rows, still 1 logical line
{
use std::io::Write;
let mut f = std::fs::OpenOptions::new()
.append(true)
.open(&path)
.unwrap();
f.write_all(b"def").unwrap();
}
std::thread::sleep(std::time::Duration::from_millis(500));
app.poll_file_watcher();
assert_eq!(app.total_lines(), 1,
"no new logical line should be added");
let vhi = app.get_visual_height_index().expect("VHI should still exist");
assert_eq!(vhi.visual_height_of_line(0), 2,
"line 0 height should update even when no new lines added");
assert_eq!(vhi.total_visual_rows(), 2);
cleanup(&path);
});
assert!(result.is_ok());
}
} }