diff --git a/crates/tui/src/app.rs b/crates/tui/src/app.rs index 075e031..0ea62c8 100644 --- a/crates/tui/src/app.rs +++ b/crates/tui/src/app.rs @@ -150,13 +150,13 @@ impl App { } pub fn load_file(&mut self, path: &str) -> anyhow::Result<()> { - // Cancel any existing background indexer by dropping the old state - self.file_watcher = None; - + // ── Phase 1: Pure computation, no mutation of self ────────── + // If any step fails and returns ?, self remains completely untouched, + // preserving the old file's watcher, loading_state, and file_path. let mut pfr = ProgressiveFileReader::open(Path::new(path)) .map_err(|e| anyhow::anyhow!("{e}"))?; - if pfr.is_sampling() { + let new_loading_state = if pfr.is_sampling() { // Cache miss: spawn background indexer let (cancel_tx, cancel_rx) = crossbeam_channel::bounded(1); let generation = pfr.generation(); @@ -175,19 +175,27 @@ impl App { ).map_err(|e| anyhow::anyhow!("{e}"))?; let estimated = pfr.line_count() as u64; - self.loading_state = AppLoadingState::Loading { + AppLoadingState::Loading { reader: pfr, estimated_lines: estimated, progress_percent: 0.0, - }; + } } else { // Cache hit: Ready state - self.loading_state = AppLoadingState::Ready { reader: pfr }; - } + AppLoadingState::Ready { reader: pfr } + }; - self.file_watcher = FileWatcher::watch(Path::new(path)).ok(); + let new_watcher = FileWatcher::watch(Path::new(path)).ok(); + // ── Phase 2: Commit — swap self to new state ─────────────── + // SAFETY: Do NOT add any fallible operations (with ?) below this point. + // The old file_watcher and loading_state are dropped here, cancelling + // any background indexer for the previous file. + self.file_watcher = new_watcher; + self.loading_state = new_loading_state; self.file_path = Some(path.to_string()); + + // Reset UI state for the new file self.cursor_line = 0; self.v_offset = 0; self.v_sub_offset = 0; @@ -2351,17 +2359,28 @@ plain text line } #[test] - fn test_file_watcher_stopped_on_nonexistent_file() { + fn test_file_watcher_preserved_on_nonexistent_file() { let path = make_temp_file("data\n"); let result = std::panic::catch_unwind(|| { let mut app = App::new(); app.load_file(path.to_str().unwrap()).unwrap(); assert!(app.file_watcher.is_some()); - let _ = app.load_file("/tmp/no_such_file_log_viewer_test_xyz_999"); + let result = app.load_file("/tmp/no_such_file_log_viewer_test_xyz_999"); + assert!(result.is_err()); + assert!( - app.file_watcher.is_none(), - "file_watcher should be None after failed load_file" + app.file_watcher.is_some(), + "file_watcher should be preserved after failed load_file" + ); + assert_eq!( + app.file_path.as_deref(), + Some(path.to_str().unwrap()), + "file_path should be preserved after failed load_file" + ); + assert!( + app.is_loaded(), + "loading_state should remain Ready after failed load_file" ); }); cleanup(&path);