From d4679a754322449b1e85d2e90395b3318d9262fe Mon Sep 17 00:00:00 2001 From: dailz Date: Sun, 7 Jun 2026 09:46:24 +0800 Subject: [PATCH] fix(io): update visual height of last line on append without trailing newline (closes #11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- crates/core/src/io/progressive_reader.rs | 106 ++++++++++++++++++++++- crates/tui/src/app.rs | 104 +++++++++++++++++++++- 2 files changed, 206 insertions(+), 4 deletions(-) diff --git a/crates/core/src/io/progressive_reader.rs b/crates/core/src/io/progressive_reader.rs index 7652ad4..782aac6 100644 --- a/crates/core/src/io/progressive_reader.rs +++ b/crates/core/src/io/progressive_reader.rs @@ -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.terminal_width = terminal_width; self @@ -159,6 +159,31 @@ impl VisualHeightIndex { 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 ──────────────────────────────────────────────── @@ -1339,6 +1364,85 @@ mod tests { 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] fn test_spawn_indexer_file_truncated_during_scan() { let mut content = Vec::new(); diff --git a/crates/tui/src/app.rs b/crates/tui/src/app.rs index cdd2f9f..cc882df 100644 --- a/crates/tui/src/app.rs +++ b/crates/tui/src/app.rs @@ -865,6 +865,7 @@ impl App { let width = self.get_content_width(); match &mut self.loading_state { AppLoadingState::Ready { reader } => { + let old_reader_line_count = reader.line_count(); let status = reader.update_for_append(); match status { Ok( @@ -883,13 +884,24 @@ impl App { }; 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 { visual_height_index: Some(index), reader: fr, } = &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 { let line_text = fr.get_line(i).unwrap_or(""); new_heights.push(compute_line_visual_height( @@ -2473,7 +2485,9 @@ plain text line } 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 log_viewer_core::io::progressive_reader::ReaderState::Ready { visual_height_index, @@ -2800,4 +2814,88 @@ plain text line cleanup(&path); 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()); + } } \ No newline at end of file