Refactor & robust terminal restore (#1)

Author: panicbit

src/event.rs

@@ -1,53 +1,56 @@
 use crate::model::{Message, Model};
+use anyhow::Context;
 use crossterm::event;
 use crossterm::event::{Event, KeyCode, KeyModifiers};
 use ratatui::prelude::Size;
 use std::time::Duration;
 
 pub fn handle_event(_: &Model) -> anyhow::Result<Option<Message>> {
-    if event::poll(Duration::from_millis(250))? {
-        match event::read()? {
-            Event::Key(key) if key.kind == event::KeyEventKind::Press => {
-                return Ok(handle_key(key));
-            }
-            Event::Resize(cols, rows) => {
-                return Ok(handle_resize(cols, rows));
-            }
-            _ => (),
-        }
+    let event_available = event::poll(Duration::from_millis(250)).context("failed to poll event")?;
+
+    if !event_available {
+        return Ok(None);
     }
-    Ok(None)
+
+    let event = event::read().context("failed to read event")?;
+    let message = match event {
+        Event::Key(key) if key.kind == event::KeyEventKind::Press => handle_key(key),
+        Event::Resize(cols, rows) => handle_resize(cols, rows),
+        _ => None,
+    };
+
+    Ok(message)
 }
 
 fn handle_key(key: event::KeyEvent) -> Option<Message> {
-    match key.modifiers {
+    Some(match key.modifiers {
         KeyModifiers::NONE => match key.code {
-            KeyCode::Home => Some(Message::First),
-            KeyCode::End => Some(Message::Last),
-            KeyCode::Up => Some(Message::ScrollUp),
-            KeyCode::Down => Some(Message::ScrollDown),
-            KeyCode::PageUp => Some(Message::PageUp),
-            KeyCode::PageDown => Some(Message::PageDown),
-            KeyCode::Left => Some(Message::ScrollLeft),
-            KeyCode::Right => Some(Message::ScrollRight),
-            KeyCode::Enter => Some(Message::Enter),
-            KeyCode::Esc => Some(Message::Exit),
-            KeyCode::Char('/') => Some(Message::OpenFindTask),
-            KeyCode::Backspace => Some(Message::Backspace),
-            KeyCode::Char(c) => Some(Message::CharacterInput(c)),
-            _ => None,
+            KeyCode::Home => Message::First,
+            KeyCode::End => Message::Last,
+            KeyCode::Up => Message::ScrollUp,
+            KeyCode::Down => Message::ScrollDown,
+            KeyCode::PageUp => Message::PageUp,
+            KeyCode::PageDown => Message::PageDown,
+            KeyCode::Left => Message::ScrollLeft,
+            KeyCode::Right => Message::ScrollRight,
+            KeyCode::Enter => Message::Enter,
+            KeyCode::Esc => Message::Exit,
+            KeyCode::Char('/') => Message::OpenFindTask,
+            KeyCode::Backspace => Message::Backspace,
+            KeyCode::Char(c) => Message::CharacterInput(c),
+            _ => return None,
         },
         KeyModifiers::SHIFT => match key.code {
-            KeyCode::Char(c) => Some(Message::CharacterInput(c)),
-            _ => None
-        }
+            KeyCode::Char(c) => Message::CharacterInput(c),
+            _ => return None,
+        },
         KeyModifiers::CONTROL => match key.code {
-            KeyCode::Char('s') => Some(Message::SaveSettings),
-            KeyCode::Char('f') => Some(Message::OpenFindTask),
-            _ => None,
+            KeyCode::Char('s') => Message::SaveSettings,
+            KeyCode::Char('f') => Message::OpenFindTask,
+            _ => return None,
         },
-        _ => None,
-    }
+        _ => return None,
+    })
 }
 
 fn handle_resize(

src/main.rs

@@ -8,11 +8,13 @@ mod terminal;
 use crate::model::{Model, Screen};
 use crate::props::Props;
 use crate::raw_json_lines::{RawJsonLines, SourceName};
-use anyhow::anyhow;
+use anyhow::{Context, anyhow};
 use clap::Parser;
+use ratatui::prelude::Backend;
+use ratatui::Terminal;
 use std::fs::File;
 use std::io;
-use std::io::{BufRead, Write};
+use std::io::BufRead;
 use std::path::{Path, PathBuf};
 
 #[derive(Parser, Debug)]
@@ -36,21 +38,35 @@ struct Args {
 
 fn main() -> anyhow::Result<()> {
     let args = Args::parse();
-    let props: Props = init_props(&args)?;
+    let props: Props = init_props(&args).context("failed to init props")?;
 
-    let lines = load_files(&args.files)?;
+    let lines = load_files(&args.files).context("failed to load files")?;
 
     terminal::install_panic_hook();
-    let mut terminal = terminal::init_terminal()?;
+    let terminal = terminal::init_terminal().context("failed to initialize terminal")?;
 
-    let mut model = Model::new(props, terminal.size().map_err(|e| anyhow!("{e}"))?, &lines);
+    if let Err(err) = run_app(terminal, props, lines) {
+        eprintln!("{err:?}");
+    }
+
+    terminal::restore_terminal().context("failed to restore terminal state")?;
+
+    Ok(())
+}
+
+fn run_app(mut terminal: Terminal<impl Backend>, props: Props, lines: RawJsonLines) -> Result<(), anyhow::Error> {
+    let terminal_size = terminal.size().map_err(|e| anyhow!("{e}")).context("failed to get terminal size")?;
+    let mut model = Model::new(props, terminal_size, &lines);
 
     while model.active_screen != Screen::Done {
         // Render the current view
-        terminal.draw(|f| terminal::view(&mut model, f)).map_err(|e| anyhow!("{e}"))?;
+        terminal
+            .draw(|f| terminal::view(&mut model, f))
+            .map_err(|e| anyhow!("{e}"))
+            .context("failed to draw to terminal")?;
 
         // Handle events and map to a Message
-        let mut current_msg = event::handle_event(&model)?;
+        let mut current_msg = event::handle_event(&model).context("failed to handle event")?;
 
         // Process updates as long as they return a non-None message
         while let Some(msg) = current_msg {
@@ -60,29 +76,32 @@ fn main() -> anyhow::Result<()> {
         }
     }
 
-    terminal::restore_terminal()?;
     Ok(())
 }
 
+
 fn init_props(args: &Args) -> anyhow::Result<Props> {
-    let mut props = Props::init()?;
+    let mut props = Props::init().context("failed to load props")?;
+
     if let Some(e) = &args.field_order {
         props.fields_order = e.clone();
     }
+
     if let Some(e) = &args.suppressed_fields {
         props.fields_suppressed = e.clone();
     }
+
     Ok(props)
 }
 
 fn load_files(files: &[PathBuf]) -> anyhow::Result<RawJsonLines> {
     let mut raw_lines = RawJsonLines::default();
-    for f in files {
-        let path = PathBuf::from(f);
-        match path.extension().map(|e| e.to_str()) {
-            Some(Some("json")) => load_lines_from_json(&mut raw_lines, &path)?,
-            Some(Some("zip")) => load_lines_from_zip(&mut raw_lines, &path)?,
-            _ => writeln!(&mut io::stderr(), "unknown file extension: '{}'", path.to_string_lossy()).expect("failed to write to stderr"),
+
+    for path in files {
+        match path.extension().and_then(|e| e.to_str()) {
+            Some("json") => load_lines_from_json(&mut raw_lines, path).with_context(|| format!("failed to load lines from {path:?}"))?,
+            Some("zip") => load_lines_from_zip(&mut raw_lines, path).with_context(|| format!("failed to load lines from {path:?}"))?,
+            _ => eprintln!("unknown file extension: '{}'", path.to_string_lossy()),
         }
     }
 
@@ -93,34 +112,56 @@ fn load_lines_from_json(
     raw_lines: &mut RawJsonLines,
     path: &Path,
 ) -> anyhow::Result<()> {
-    for (line_nr, line) in io::BufReader::new(File::open(path)?).lines().enumerate() {
-        raw_lines.push(SourceName::JsonFile(path.file_name().unwrap().to_string_lossy().into()), line_nr + 1, line?);
+    let json_file = File::open(path).context("failed to open json")?;
+    let json_file = io::BufReader::new(json_file);
+
+    for (line_nr, line) in json_file.lines().enumerate() {
+        let line = line.context("failed to read json line")?;
+        let file_name = path
+            .file_name()
+            .context("BUG: json path is missing filename")?
+            .to_string_lossy()
+            .into();
+        let source_name = SourceName::JsonFile(file_name);
+
+        raw_lines.push(source_name, line_nr + 1, line);
     }
+
     Ok(())
 }
 
 fn load_lines_from_zip(
     raw_lines: &mut RawJsonLines,
     path: &Path,
 ) -> anyhow::Result<()> {
-    let zip_file = File::open(path)?;
-    let mut archive = zip::ZipArchive::new(zip_file)?;
+    let zip_file = File::open(path).context("failed to open zip")?;
+    let mut archive = zip::ZipArchive::new(zip_file).context("failed to parse zip")?;
 
     for i in 0..archive.len() {
-        let f = archive.by_index(i)?;
-        if f.is_file() && f.name().ends_with(".json") {
-            let json_file = f.name().to_string();
-            for (line_nr, line) in io::BufReader::new(f).lines().enumerate() {
-                raw_lines.push(
-                    SourceName::JsonInZip {
-                        zip_file: path.file_name().unwrap().to_string_lossy().into(),
-                        json_file: json_file.clone(),
-                    },
-                    line_nr + 1,
-                    line?,
-                );
-            }
+        let f = archive
+            .by_index(i)
+            .with_context(|| format!("failed to get file with index {i} from zip"))?;
+
+        if !f.is_file() || !f.name().ends_with(".json") {
+            continue;
+        }
+
+        let json_file = f.name().to_string();
+        let f = io::BufReader::new(f);
+
+        for (line_nr, line) in f.lines().enumerate() {
+            let line = line.context("failed to read line from file in zip")?;
+            let zip_file = path
+                .file_name()
+                .context("BUG: zip path is missing filename")?
+                .to_string_lossy()
+                .into();
+            let json_file = json_file.clone();
+            let source_name = SourceName::JsonInZip { zip_file, json_file };
+
+            raw_lines.push(source_name, line_nr + 1, line);
         }
     }
+
     Ok(())
 }

src/model.rs

@@ -8,7 +8,6 @@ use std::cell::Cell;
 use std::cmp;
 use std::num::NonZero;
 use std::ops::Add;
-use std::rc::Rc;
 
 #[derive(Clone)]
 pub struct Model<'a> {
@@ -372,47 +371,57 @@ impl<'a> Model<'a> {
     }
 
     pub fn render_status_line_left(&self) -> String {
-        match self.view_state.main_window_list_state.selected() {
-            Some(line_nr) if self.raw_json_lines.lines.len() > line_nr => {
-                let raw_line = &self.raw_json_lines.lines[line_nr];
-                let source_name = self.raw_json_lines.source_name(raw_line.source_id).expect("invalid source id");
-                format!("{}:{}", source_name, raw_line.line_nr)
-            }
-            _ => String::new(),
-        }
+        let Some(line_nr) = self.view_state.main_window_list_state.selected() else {
+            return "".into();
+        };
+
+        let Some(raw_line) = self.raw_json_lines.lines.get(line_nr) else {
+            return "".into();
+        };
+
+        let source_name = self.raw_json_lines.source_name(raw_line.source_id).expect("invalid source id");
+
+        format!("{}:{}", source_name, raw_line.line_nr)
     }
+
     pub fn render_status_line_right(&self) -> String {
         self.last_action_result.clone()
     }
 
     pub fn render_find_task_line_left(&self) -> Line {
-        if let Some(task) = self.find_task.as_ref() {
-            let color = match task.found {
-                None => Color::default(),
-                Some(false) => Color::Red,
-                Some(true) => Color::Green
-            };
-            " [".to_span().set_style(color)
-                .add("Find ".to_span())
-                .add("🔍".to_span())
-                .add(": ".bold())
-                .add(task.search_string.to_span().bold())
-                .add("  ] ".to_span().set_style(color)).to_owned()
-        } else {
-            Line::raw("").to_owned()
-        }
+        let Some(task) = &self.find_task else {
+            return "".into();
+        };
+
+        let color = match task.found {
+            None => Color::default(),
+            Some(false) => Color::Red,
+            Some(true) => Color::Green,
+        };
+
+        " [".to_span()
+            .set_style(color)
+            .add("Find ".to_span())
+            .add("🔍".to_span())
+            .add(": ".bold())
+            .add(task.search_string.to_span().bold())
+            .add("  ] ".to_span().set_style(color))
+            .to_owned()
     }
 
     pub fn render_find_task_line_right(&self) -> Line {
-        if let Some(t) = self.find_task.as_ref() {
-            if let Some(state) = t.found {
-                return match state {
-                    true => "found".to_owned().into(),
-                    false => "NOT found".to_owned().into(),
-                }
-            }
+        let Some(task) = &self.find_task else {
+            return "".into();
+        };
+
+        let Some(found) = task.found else {
+            return "".into();
+        };
+
+        match found {
+            true => "found".into(),
+            false => "NOT found".into(),
         }
-        "".into()
     }
 
     pub fn page_len(&self) -> u16 {
@@ -512,7 +521,7 @@ pub struct ModelIntoIter<'a> {
     index: usize,
 }
 
-impl<'a> ModelIntoIter<'a> {
+impl ModelIntoIter<'_> {
     // light version of Self::next() that simply skips the item.
     // returns true if the item was skipped, false if there are no more items
     fn skip_item(&mut self) -> bool {
@@ -538,19 +547,15 @@ impl<'a> Iterator for ModelIntoIter<'a> {
     type Item = ListItem<'a>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        if self.index >= self.model.raw_json_lines.lines.len() {
-            None
-        } else {
-            let raw_line = &self.model.raw_json_lines.lines[self.index];
-            let json: Rc<serde_json::Value> = Rc::new(serde_json::from_str(&raw_line.content).expect("invalid json"));
-            let line = match json.as_ref() {
-                serde_json::Value::Object(o) => self.model.render_json_line(o),
-                e => Line::from(format!("{e}")),
-            };
+        let raw_line = self.model.raw_json_lines.lines.get(self.index)?;
+        let json = serde_json::from_str::<serde_json::Value>(&raw_line.content).expect("invalid json");
+        let line = match json {
+            serde_json::Value::Object(o) => self.model.render_json_line(&o),
+            e => Line::from(format!("{e}")),
+        };
 
-            self.index += 1;
-            Some(ListItem::new(line))
-        }
+        self.index += 1;
+        Some(ListItem::new(line))
     }
 
     fn size_hint(&self) -> (usize, Option<usize>) {