Skip to content

Commit

Permalink
Cleaned up errors in Jolly (#32)
Browse files Browse the repository at this point in the history
Errors now properly show the correct filename.

Toml Errors no longer show the gutter display that required a fixed point font.
  • Loading branch information
apgoetz committed Aug 6, 2023
1 parent 92fac67 commit b1e3d01
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 35 deletions.
11 changes: 7 additions & 4 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,15 @@ fn get_logfile() -> Result<path::PathBuf, Error> {
}

pub fn load_path<P: AsRef<path::Path>>(path: P) -> Result<Config, Error> {
let txt = fs::read_to_string(path).map_err(Error::IoError)?;
let txt = fs::read_to_string(&path)
.map_err(|e| Error::IoError(Some(path.as_ref().display().to_string()), e))?;
load_txt(&txt)
.map_err(|e| Error::ContextParseError(path.as_ref().display().to_string(), e.to_string()))
}

fn load_txt(txt: &str) -> Result<Config, Error> {
let value: toml::Value = toml::from_str(txt).map_err(|e| Error::ParseError(e.to_string()))?;
let value: toml::Value =
toml::from_str(txt).map_err(|e| Error::ParseError(e.message().to_string()))?;

let mut parsed_config = match value {
toml::Value::Table(t) => t,
Expand All @@ -117,7 +120,7 @@ fn load_txt(txt: &str) -> Result<Config, Error> {

let mut settings = match parsed_config.remove("config") {
Some(config) => {
Settings::deserialize(config).map_err(|e| Error::ParseError(e.to_string()))?
Settings::deserialize(config).map_err(|e| Error::ParseError(e.message().to_string()))?
}
None => Settings::default(),
};
Expand Down Expand Up @@ -179,7 +182,7 @@ mod tests {
#[test]
fn nonexistent_path() {
let result = load_path("nonexistentfile.toml");
assert!(matches!(result, Err(Error::IoError(_))));
assert!(matches!(result, Err(Error::IoError(_, _))));
}

#[test]
Expand Down
29 changes: 7 additions & 22 deletions src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ use crate::theme;
use crate::ui;
use crate::{icon, platform};

use crate::config::LOGFILE_NAME;

// these are the weights for the different kind of matches.
// we prefer each weight to be different so we can differentiate them in the test plan
const FULL_KEYWORD_W: u32 = 100;
Expand All @@ -28,30 +26,15 @@ pub type EntryId = usize;

#[derive(Debug)]
pub enum Error {
IOError(std::io::Error),
ParseError(String),
BareKeyError(String),
CustomError(String),
PlatformError(platform::Error),
}

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Error::IOError(e) => e.fmt(f),
Error::PlatformError(e) => e.fmt(f),
Error::BareKeyError(e) => {
write!(
f,
"Invalid {} entry '{}': Jolly entries can only be TOML tables",
LOGFILE_NAME, e
)
}
Error::ParseError(e) => {
write!(f, "TOML Error: ")?;
e.fmt(f)
}
Error::CustomError(s) => f.write_str(s),
Error::ParseError(s) => f.write_str(s),
}
}
}
Expand Down Expand Up @@ -134,11 +117,13 @@ impl StoreEntry {
// parse a toml value into a store entry
pub fn from_value(name: String, val: toml::Value) -> Result<Self, Error> {
if !val.is_table() {
return Err(Error::BareKeyError(name));
return Err(Error::ParseError(format!(
"Invalid entry '{name}': Jolly entries can only be TOML tables"
)));
}

let raw_entry =
RawStoreEntry::deserialize(val).map_err(|e| Error::ParseError(e.to_string()))?;
let raw_entry = RawStoreEntry::deserialize(val)
.map_err(|e| Error::ParseError(format!("TOML Error: {}", e.message())))?;

let keyword = if let Some(keyword) = raw_entry.keyword {
if raw_entry.url.is_some() || raw_entry.escape.unwrap_or(false) {
Expand All @@ -158,7 +143,7 @@ impl StoreEntry {
(None, None, Some(loc)) => loc,
(None, None, None) => name.to_string(),
_ => {
return Err(Error::CustomError(format!(
return Err(Error::ParseError(format!(
"Error with entry ['{}']: The entry should only specify one of location/url/system keys",
&name
)))
Expand Down
17 changes: 12 additions & 5 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ use std::io;
pub enum Error {
StoreError(entry::Error),
IcedError(iced::Error),
IoError(io::Error),
IoError(Option<String>, io::Error),
ParseError(String),
ContextParseError(String, String),
PlatformError(platform::Error),
CustomError(String),
FinalMessage(String),
Expand All @@ -25,15 +26,21 @@ impl fmt::Display for Error {
e.fmt(f)
}
Error::IcedError(e) => e.fmt(f),
Error::IoError(e) => {
write!(f, "could not access jolly.toml: \n")?;
Error::IoError(file, e) => {
if let Some(file) = file {
write!(f, "with file '{file}': \n")?;
} else {
f.write_str("IO Error:\n")?;
}
e.fmt(f)
}
Error::ParseError(e) => {
write!(f, "while parsing jolly.toml: \n")?;
Error::ParseError(e) => f.write_str(e),
Error::ContextParseError(file, e) => {
write!(f, "while parsing '{file}':\n")?;
e.fmt(f)
}
Error::PlatformError(e) => e.fmt(f),

Error::CustomError(s) => f.write_str(s),
// not really an error, used to represent final message in UI
Error::FinalMessage(s) => f.write_str(s),
Expand Down
6 changes: 3 additions & 3 deletions src/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ impl LogSettings {
.format_timestamp_micros()
.target(env_logger::fmt::Target::Stderr);

if let Some(f) = &self.file {
if let Some(fname) = &self.file {
let f = std::fs::OpenOptions::new()
.append(true)
.create(true)
.open(f)
.map_err(error::Error::IoError)?;
.open(fname)
.map_err(|e| error::Error::IoError(self.file.clone(), e))?;
builder.target(env_logger::fmt::Target::Pipe(Box::new(f)));
}

Expand Down
2 changes: 1 addition & 1 deletion src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ pub mod tests {
let toml = r#"bare_key = 42"#;
let text = parse_store(toml);
assert!(
matches!(text, Err(entry::Error::BareKeyError(_))),
matches!(text, Err(entry::Error::ParseError(_))),
"{:?}",
text
)
Expand Down

0 comments on commit b1e3d01

Please sign in to comment.