fix(tui): defer file-change events during Loading state to prevent stale reader (closes #10)
Loading state silently dropped FileEvent::Appended/Truncated via _ => {}.
After Loading→Ready transition the FileReader was based on a stale snapshot.
- Add reload_after_loading flag to defer reload until Ready state
- Extract reload_ready_reader() from handle_file_truncated
- Explicit 3-branch match: Ready handles, Loading sets flag, rest ignores
- Clear flag on IndexerMessage::Error to prevent stale dirty bit
- 4 regression tests covering append/truncate/collapse/error paths
This commit is contained in:
@@ -117,6 +117,10 @@ pub struct App {
|
|||||||
|
|
||||||
// File watcher
|
// File watcher
|
||||||
file_watcher: Option<FileWatcher>,
|
file_watcher: Option<FileWatcher>,
|
||||||
|
|
||||||
|
/// Set to true when file-change events arrive during Loading state.
|
||||||
|
/// After Loading→Ready transition, a full reload is triggered once.
|
||||||
|
reload_after_loading: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl App {
|
impl App {
|
||||||
@@ -138,6 +142,7 @@ impl App {
|
|||||||
settings_cursor: 0,
|
settings_cursor: 0,
|
||||||
settings_draft: ColorConfig::default(),
|
settings_draft: ColorConfig::default(),
|
||||||
file_watcher: None,
|
file_watcher: None,
|
||||||
|
reload_after_loading: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -184,9 +189,10 @@ impl App {
|
|||||||
self.v_offset = 0;
|
self.v_offset = 0;
|
||||||
self.v_sub_offset = 0;
|
self.v_sub_offset = 0;
|
||||||
self.viewport_cache.invalidate();
|
self.viewport_cache.invalidate();
|
||||||
self.last_g_press = None; // reset gg state machine
|
self.last_g_press = None;
|
||||||
self.json_format = false;
|
self.json_format = false;
|
||||||
self.mode = AppMode::Normal;
|
self.mode = AppMode::Normal;
|
||||||
|
self.reload_after_loading = false;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -913,22 +919,34 @@ impl App {
|
|||||||
Ok(log_viewer_core::io::file_reader::AppendStatus::Unchanged) | Err(_) => {}
|
Ok(log_viewer_core::io::file_reader::AppendStatus::Unchanged) | Err(_) => {}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
AppLoadingState::Loading { .. } => {
|
||||||
|
self.reload_after_loading = true;
|
||||||
|
}
|
||||||
_ => {}
|
_ => {}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn handle_file_truncated(&mut self) {
|
fn reload_ready_reader(&mut self) {
|
||||||
let width = self.get_content_width();
|
let width = self.get_content_width();
|
||||||
|
if let AppLoadingState::Ready { reader } = &mut self.loading_state {
|
||||||
|
let _ = reader.reload();
|
||||||
|
let _ = reader.save_cache();
|
||||||
|
reader.invalidate_visual_height_index();
|
||||||
|
reader.start_visual_height_rebuild(width, self.json_format);
|
||||||
|
self.cursor_line = self.cursor_line.min(self.total_lines().saturating_sub(1));
|
||||||
|
self.v_sub_offset = 0;
|
||||||
|
self.viewport_cache.invalidate();
|
||||||
|
self.clamp_v_offset();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn handle_file_truncated(&mut self) {
|
||||||
match &mut self.loading_state {
|
match &mut self.loading_state {
|
||||||
AppLoadingState::Ready { reader } => {
|
AppLoadingState::Ready { .. } => {
|
||||||
let _ = reader.reload();
|
self.reload_ready_reader();
|
||||||
let _ = reader.save_cache();
|
}
|
||||||
reader.invalidate_visual_height_index();
|
AppLoadingState::Loading { .. } => {
|
||||||
reader.start_visual_height_rebuild(width, self.json_format);
|
self.reload_after_loading = true;
|
||||||
self.cursor_line = self.cursor_line.min(self.total_lines().saturating_sub(1));
|
|
||||||
self.v_sub_offset = 0;
|
|
||||||
self.viewport_cache.invalidate();
|
|
||||||
self.clamp_v_offset();
|
|
||||||
}
|
}
|
||||||
_ => {}
|
_ => {}
|
||||||
}
|
}
|
||||||
@@ -998,9 +1016,15 @@ impl App {
|
|||||||
if let AppLoadingState::Ready { reader } = &mut self.loading_state {
|
if let AppLoadingState::Ready { reader } = &mut self.loading_state {
|
||||||
reader.invalidate_visual_height_index();
|
reader.invalidate_visual_height_index();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if self.reload_after_loading {
|
||||||
|
self.reload_after_loading = false;
|
||||||
|
self.reload_ready_reader();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
IndexerMessage::Error { message, .. } => {
|
IndexerMessage::Error { message, .. } => {
|
||||||
self.loading_state = AppLoadingState::Error(message);
|
self.loading_state = AppLoadingState::Error(message);
|
||||||
|
self.reload_after_loading = false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
@@ -2671,4 +2695,109 @@ plain text line
|
|||||||
}));
|
}));
|
||||||
assert!(result.is_ok());
|
assert!(result.is_ok());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── H10 regression: file events during Loading state ──────────────
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_append_during_loading_sets_reload_flag() {
|
||||||
|
let content: String = (0..50)
|
||||||
|
.map(|i| format!("line {}\n", i))
|
||||||
|
.collect();
|
||||||
|
let path = make_temp_file(&content);
|
||||||
|
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||||
|
let (mut app, _tx) = app_in_loading_state(&path, 1);
|
||||||
|
assert!(app.is_loading());
|
||||||
|
assert!(!app.reload_after_loading);
|
||||||
|
|
||||||
|
app.handle_file_appended();
|
||||||
|
|
||||||
|
assert!(app.reload_after_loading,
|
||||||
|
"append during Loading should set reload_after_loading flag");
|
||||||
|
}));
|
||||||
|
cleanup(&path);
|
||||||
|
assert!(result.is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_truncate_during_loading_sets_reload_flag() {
|
||||||
|
let content: String = (0..50)
|
||||||
|
.map(|i| format!("line {}\n", i))
|
||||||
|
.collect();
|
||||||
|
let path = make_temp_file(&content);
|
||||||
|
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||||
|
let (mut app, _tx) = app_in_loading_state(&path, 1);
|
||||||
|
assert!(app.is_loading());
|
||||||
|
|
||||||
|
app.handle_file_truncated();
|
||||||
|
|
||||||
|
assert!(app.reload_after_loading,
|
||||||
|
"truncate during Loading should set reload_after_loading flag");
|
||||||
|
}));
|
||||||
|
cleanup(&path);
|
||||||
|
assert!(result.is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_multiple_events_during_loading_collapse_to_single_reload() {
|
||||||
|
let content: String = (0..50)
|
||||||
|
.map(|i| format!("line {}\n", i))
|
||||||
|
.collect();
|
||||||
|
let path = make_temp_file(&content);
|
||||||
|
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||||
|
let (mut app, tx) = app_in_loading_state(&path, 1);
|
||||||
|
|
||||||
|
app.handle_file_appended();
|
||||||
|
app.handle_file_truncated();
|
||||||
|
app.handle_file_appended();
|
||||||
|
|
||||||
|
assert!(app.reload_after_loading);
|
||||||
|
|
||||||
|
let updated_content: String = (0..60)
|
||||||
|
.map(|i| format!("updated line {}\n", i))
|
||||||
|
.collect();
|
||||||
|
std::fs::write(&path, &updated_content).unwrap();
|
||||||
|
let fr = file_reader_for(&path);
|
||||||
|
tx.send(IndexerMessage::Complete {
|
||||||
|
generation: 1,
|
||||||
|
reader: fr,
|
||||||
|
visual_height_index: None,
|
||||||
|
}).unwrap();
|
||||||
|
|
||||||
|
app.poll_background_indexer();
|
||||||
|
|
||||||
|
assert!(!app.is_loading(), "should be Ready after Complete");
|
||||||
|
assert!(!app.reload_after_loading, "flag should be cleared");
|
||||||
|
assert_eq!(app.total_lines(), 60,
|
||||||
|
"should show reloaded content, not stale indexer result");
|
||||||
|
}));
|
||||||
|
cleanup(&path);
|
||||||
|
assert!(result.is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_indexer_error_clears_reload_flag() {
|
||||||
|
let content: String = (0..10)
|
||||||
|
.map(|i| format!("line {}\n", i))
|
||||||
|
.collect();
|
||||||
|
let path = make_temp_file(&content);
|
||||||
|
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
||||||
|
let (mut app, tx) = app_in_loading_state(&path, 1);
|
||||||
|
|
||||||
|
app.handle_file_appended();
|
||||||
|
assert!(app.reload_after_loading);
|
||||||
|
|
||||||
|
tx.send(IndexerMessage::Error {
|
||||||
|
generation: 1,
|
||||||
|
message: "test error".into(),
|
||||||
|
}).unwrap();
|
||||||
|
|
||||||
|
app.poll_background_indexer();
|
||||||
|
|
||||||
|
assert!(matches!(app.loading_state, AppLoadingState::Error(_)));
|
||||||
|
assert!(!app.reload_after_loading,
|
||||||
|
"flag should be cleared on indexer error");
|
||||||
|
}));
|
||||||
|
cleanup(&path);
|
||||||
|
assert!(result.is_ok());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Reference in New Issue
Block a user