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
This commit is contained in:
@@ -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();
|
||||||
|
|||||||
@@ -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());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Reference in New Issue
Block a user