Overhaul file erorr handling

src/cli.rs

@@ -1,11 +1,12 @@
 use std::path::PathBuf;
+use std::{process, result};
 
 use chrono::{Duration, NaiveDate};
 use directories::ProjectDirs;
 use structopt::StructOpt;
 
 use crate::eval::{DateRange, EntryMode};
-use crate::files::Files;
+use crate::files::{self, Files};
 
 use self::error::{Error, Result};
 
@@ -56,11 +57,22 @@ fn default_file() -> PathBuf {
         .join("main.today")
 }
 
+fn load_files(opt: &Opt) -> result::Result<Files, files::Error> {
+    let file = opt.file.clone().unwrap_or_else(default_file);
+    Files::load(&file)
+}
+
 pub fn run() -> Result<()> {
     let opt = Opt::from_args();
 
-    let file = opt.file.unwrap_or_else(default_file);
-    let mut files = Files::load(&file)?;
+    let mut files = match load_files(&opt) {
+        Ok(result) => result,
+        Err(e) => {
+            e.print();
+            process::exit(1);
+        }
+    };
+
     let now = files.now().naive_local();
 
     let range_date = opt.date.unwrap_or_else(|| now.date());
@@ -80,11 +92,15 @@ pub fn run() -> Result<()> {
         },
         Some(Command::Done) => match opt.entry {
             None => return Err(Error::NoNumber),
-            Some(n) => done::mark_done(&mut files, &entries[layout.look_up_number(n)?], now)?,
+            Some(n) => done::mark_done(&mut files, &entries, &layout, n, now)?,
         },
         Some(Command::Fmt) => files.mark_all_dirty(),
     }
 
-    files.save()?;
+    if let Err(e) = files.save() {
+        e.print();
+        process::exit(1);
+    }
+
     Ok(())
 }

src/cli/done.rs

@@ -5,12 +5,20 @@ use crate::files::commands::Done;
 use crate::files::Files;
 
 use super::error::Result;
+use super::layout::line::LineLayout;
 
-pub fn mark_done(files: &mut Files, entry: &Entry, now: NaiveDateTime) -> Result<()> {
+pub fn mark_done(
+    files: &mut Files,
+    entries: &[Entry],
+    layout: &LineLayout,
+    number: usize,
+    now: NaiveDateTime,
+) -> Result<()> {
+    let entry = &entries[layout.look_up_number(number)?];
     let done = Done {
         date: entry.dates.map(|dates| dates.into()),
         done_at: now.date(),
     };
-    files.add_done(entry.source, done)?;
+    files.add_done(number, entry.source, done)?;
     Ok(())
 }

src/files.rs

@@ -1,13 +1,15 @@
 use std::collections::HashMap;
+use std::fs;
 use std::path::{Path, PathBuf};
-use std::{fs, io, result};
 
 use chrono::{DateTime, Utc};
 use tzfile::Tz;
 
 use self::commands::{Command, Done, File};
+pub use self::error::{Error, Result};
 
 pub mod commands;
+mod error;
 mod format;
 mod parse;
 pub mod primitives;
@@ -58,34 +60,6 @@ pub struct Files {
     timezone: Tz,
 }
 
-#[derive(Debug, thiserror::Error)]
-pub enum Error {
-    #[error("Could not resolve {path}: {error}")]
-    ResolvePath { path: PathBuf, error: io::Error },
-    #[error("Could not load {file}: {error}")]
-    ReadFile { file: PathBuf, error: io::Error },
-    #[error("Could not write {file}: {error}")]
-    WriteFile { file: PathBuf, error: io::Error },
-    #[error("Could not resolve timezone {timezone}: {error}")]
-    ResolveTz { timezone: String, error: io::Error },
-    #[error("Could not determine local timezone: {error}")]
-    LocalTz { error: io::Error },
-    #[error("{0}")]
-    Parse(#[from] parse::Error),
-    #[error("{file1} has time zone {tz1} but {file2} has time zone {tz2}")]
-    TzConflict {
-        file1: PathBuf,
-        tz1: String,
-        file2: PathBuf,
-        tz2: String,
-    },
-    // TODO Add span or something similar for a nicer error message
-    #[error("Not a task")]
-    NotATask,
-}
-
-pub type Result<T> = result::Result<T, Error>;
-
 impl Files {
     pub fn load(path: &Path) -> Result<Self> {
         let mut paths = HashMap::new();
@@ -189,11 +163,11 @@ impl Files {
         &self.files[source.file].file.commands[source.command]
     }
 
-    pub fn add_done(&mut self, source: Source, done: Done) -> Result<()> {
+    pub fn add_done(&mut self, number: usize, source: Source, done: Done) -> Result<()> {
         let file = &mut self.files[source.file];
         match &mut file.file.commands[source.command] {
             Command::Task(t) => t.done.push(done),
-            Command::Note(_) => return Err(Error::NotATask),
+            Command::Note(_) => return Err(Error::NotATask(vec![number])),
         }
         file.dirty = true;
         Ok(())

src/files/error.rs

@@ -0,0 +1,79 @@
+use std::path::PathBuf;
+use std::{io, result};
+
+use super::parse;
+
+#[derive(Debug, thiserror::Error)]
+pub enum Error {
+    #[error("Could not resolve {path}: {error}")]
+    ResolvePath { path: PathBuf, error: io::Error },
+    #[error("Could not load {file}: {error}")]
+    ReadFile { file: PathBuf, error: io::Error },
+    #[error("Could not write {file}: {error}")]
+    WriteFile { file: PathBuf, error: io::Error },
+    #[error("Could not resolve timezone {timezone}: {error}")]
+    ResolveTz { timezone: String, error: io::Error },
+    #[error("Could not determine local timezone: {error}")]
+    LocalTz { error: io::Error },
+    #[error("{0}")]
+    Parse(#[from] parse::Error),
+    #[error("{file1} has time zone {tz1} but {file2} has time zone {tz2}")]
+    TzConflict {
+        file1: PathBuf,
+        tz1: String,
+        file2: PathBuf,
+        tz2: String,
+    },
+    #[error("Not a task")]
+    NotATask(Vec<usize>),
+}
+
+impl Error {
+    pub fn print(self) {
+        match self {
+            Error::ResolvePath { path, error } => {
+                eprintln!("Could not resolve path {:?}:", path);
+                eprintln!("  {}", error);
+            }
+            Error::ReadFile { file, error } => {
+                eprintln!("Could not read file {:?}:", file);
+                eprintln!("  {}", error);
+            }
+            Error::WriteFile { file, error } => {
+                eprintln!("Could not write file {:?}:", file);
+                eprintln!("  {}", error);
+            }
+            Error::ResolveTz { timezone, error } => {
+                eprintln!("Could not resolve time zone {}:", timezone);
+                eprintln!("  {}", error);
+            }
+            Error::LocalTz { error } => {
+                eprintln!("Could not determine local timezone:");
+                eprintln!("  {}", error);
+            }
+            Error::Parse(error) => eprintln!("{}", error),
+            Error::TzConflict {
+                file1,
+                tz1,
+                file2,
+                tz2,
+            } => {
+                eprintln!("Time zone conflict:");
+                eprintln!("  {:?} has time zone {}", file1, tz1);
+                eprintln!("  {:?} has time zone {}", file2, tz2);
+            }
+            Error::NotATask(numbers) => {
+                if numbers.is_empty() {
+                    eprintln!("Not a task.");
+                } else if numbers.len() == 1 {
+                    eprintln!("{} is not a task.", numbers[0]);
+                } else {
+                    let numbers = numbers.iter().map(|n| n.to_string()).collect::<Vec<_>>();
+                    eprintln!("{} are not tasks.", numbers.join(", "));
+                }
+            }
+        }
+    }
+}
+
+pub type Result<T> = result::Result<T, Error>;