From e0373890fe7c87d3c65598f999ff88a509f7aa1a Mon Sep 17 00:00:00 2001
From: Thomas Frans <franske2000@gmail.com>
Date: Tue, 28 Nov 2023 21:25:18 +0100
Subject: [PATCH] feat: improve configuration file error handling

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.
---
 CHANGELOG.md    |  1 +
 src/commands.rs | 15 +++++++++++++--
 src/config.rs   | 23 ++++++++++++++++++-----
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0f5a6bdae..5dcfdb04c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 
diff --git a/src/commands.rs b/src/commands.rs
index 24c586eb0..8c4ab3999 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -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;
@@ -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 {
@@ -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();
diff --git a/src/config.rs b/src/config.rs
index f2f8ef178..e3e7cca3c 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -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};
@@ -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);
         });
 
@@ -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(())
     }
 }