Skip to content

Commit

Permalink
feat: improve configuration file error handling
Browse files Browse the repository at this point in the history
Cleans up the error messages generated when errors are encountered in
the configuration file. Instead of showing the raw error message, give
clear information about the problem.
  • Loading branch information
ThomasFrans authored and hrkfdn committed Nov 29, 2023
1 parent 0cee99b commit e037389
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Improve error messages generated by the command line
- Build with crossterm terminal backend by default
- Move UNIX IPC socket from the user's cache path to the user's runtime directory
- Improve messages relating to errors in the configuration file

### Fixed

Expand Down
15 changes: 13 additions & 2 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::application::UserData;
use crate::command::{
parse, Command, GotoMode, JumpMode, MoveAmount, MoveMode, SeekDirection, ShiftMode, TargetMode,
};
use crate::config::Config;
use crate::config::{user_configuration_directory, Config};
use crate::events::EventManager;
use crate::ext_traits::CursiveExt;
use crate::library::Library;
Expand All @@ -25,6 +25,7 @@ use cursive::traits::View;
use cursive::views::Dialog;
use cursive::Cursive;
use log::{debug, error, info};
use ncspot::CONFIGURATION_FILE_NAME;
use std::cell::RefCell;

pub enum CommandResult {
Expand Down Expand Up @@ -213,7 +214,17 @@ impl CommandManager {
Ok(None)
}
Command::ReloadConfig => {
self.config.reload();
self.config.reload().map_err(|_| {
format!(
"Failed to reload configuration. Fix errors in {} and try again.",
user_configuration_directory()
.map(|ref mut path| {
path.push(CONFIGURATION_FILE_NAME);
path.to_string_lossy().to_string()
})
.expect("configuration directory expected but not found")
)
})?;

// update theme
let theme = self.config.build_theme();
Expand Down
23 changes: 18 additions & 5 deletions src/config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::HashMap;
use std::error::Error;
use std::path::PathBuf;
use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
use std::{fs, process};
Expand Down Expand Up @@ -195,7 +196,15 @@ impl Config {
pub fn new(filename: Option<String>) -> Self {
let filename = filename.unwrap_or(CONFIGURATION_FILE_NAME.to_owned());
let values = load(&filename).unwrap_or_else(|e| {
eprintln!("could not load config: {e}");
eprint!(
"There is an error in your configuration file at {}:\n\n{e}",
user_configuration_directory()
.map(|ref mut path| {
path.push(CONFIGURATION_FILE_NAME);
path.to_string_lossy().to_string()
})
.expect("configuration directory expected but not found")
);
process::exit(1);
});

Expand Down Expand Up @@ -256,10 +265,14 @@ impl Config {
crate::theme::load(theme)
}

/// Reload the configuration file.
pub fn reload(&self) {
let cfg = load(&self.filename).expect("could not reload config");
*self.values.write().expect("can't writelock config values") = cfg
/// Attempt to reload the configuration from the configuration file.
///
/// This only updates the values stored in memory but doesn't perform any additional actions
/// like updating active keybindings.
pub fn reload(&self) -> Result<(), Box<dyn Error>> {
let cfg = load(&self.filename)?;
*self.values.write().unwrap() = cfg;
Ok(())
}
}

Expand Down

0 comments on commit e037389

Please sign in to comment.