Refactor edit subcommand to consider the case of handling corrupted configuration files

src/config/mod.rs

@@ -68,13 +68,6 @@ fn default_exclude_default_entries() -> bool {false}
 fn default_matches() -> Vec<Match> { Vec::new() }
 fn default_global_vars() -> Vec<MatchVariable> { Vec::new() }
 
-#[cfg(target_os = "linux")]
-fn default_editor() -> String{ "/bin/nano".to_owned() }
-#[cfg(target_os = "macos")]
-fn default_editor() -> String{ "/usr/bin/nano".to_owned() } // TODO: change
-#[cfg(target_os = "windows")]
-fn default_editor() -> String{ "C:\\Windows\\System32\\notepad.exe".to_owned() }
-
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct Configs {
     #[serde(default = "default_name")]
@@ -155,9 +148,6 @@ pub struct Configs {
     #[serde(default = "default_exclude_default_entries")]
     pub exclude_default_entries: bool,
 
-    #[serde(default = "default_editor")]
-    pub editor: String,
-
     #[serde(default = "default_matches")]
     pub matches: Vec<Match>,
 

src/edit.rs

@@ -17,27 +17,33 @@
  * along with espanso.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-use crate::config::ConfigSet;
 use std::path::Path;
 
-pub fn open_editor(config: &ConfigSet, file_path: &Path) -> bool {
+#[cfg(target_os = "linux")]
+fn default_editor() -> String{ "/bin/nano".to_owned() }
+#[cfg(target_os = "macos")]
+fn default_editor() -> String{ "/usr/bin/nano".to_owned() }
+#[cfg(target_os = "windows")]
+fn default_editor() -> String{ "C:\\Windows\\System32\\notepad.exe".to_owned() }
+
+pub fn open_editor(file_path: &Path) -> bool {
     use std::process::Command;
 
     // Check if another editor is defined in the environment variables
     let editor_var = std::env::var_os("EDITOR");
     let visual_var = std::env::var_os("VISUAL");
 
-    // Prioritize the editors specified by the environment variable, otherwise use the config
+    // Prioritize the editors specified by the environment variable, use the default one
     let editor : String = if let Some(editor_var) = editor_var {
         editor_var.to_string_lossy().to_string()
     }else if let Some(visual_var) = visual_var {
         visual_var.to_string_lossy().to_string()
     }else{
-        config.default.editor.clone()
+        default_editor()
     };
 
     // Start the editor and wait for its termination
-    let status = Command::new(editor)
+    let status = Command::new(&editor)
         .arg(file_path)
         .spawn();
 
@@ -51,7 +57,7 @@ pub fn open_editor(config: &ConfigSet, file_path: &Path) -> bool {
             false
         }
     }else{
-        println!("Error: could not start editor at: {}", config.default.editor);
+        println!("Error: could not start editor at: {}", &editor);
         false
     }
 }

src/main.rs

@@ -151,6 +151,14 @@ fn main() {
 
     let matches = clap_instance.clone().get_matches();
 
+    // The edit subcommand must be run before the configuration parsing. Otherwise, if the
+    // configuration is corrupted, the edit command won't work, which makes it pretty useless.
+    if let Some(matches) = matches.subcommand_matches("edit") {
+        edit_main(matches);
+        return;
+    }
+
+
     let log_level = matches.occurrences_of("v") as i32;
 
     // Load the configuration
@@ -161,18 +169,13 @@ fn main() {
 
     config_set.default.log_level = log_level;
 
-    // Match the correct subcommand
+    // Commands that require the configuration
 
     if let Some(matches) = matches.subcommand_matches("cmd") {
         cmd_main(config_set, matches);
         return;
     }
 
-    if let Some(matches) = matches.subcommand_matches("edit") {
-        edit_main(config_set, matches);
-        return;
-    }
-
     if matches.subcommand_matches("dump").is_some() {
         println!("{:#?}", config_set);
         return;
@@ -883,7 +886,7 @@ fn path_main(_config_set: ConfigSet, matches: &ArgMatches) {
     }
 }
 
-fn edit_main(config_set: ConfigSet, matches: &ArgMatches) {
+fn edit_main(matches: &ArgMatches) {
     // Determine which is the file to edit
     let config = matches.value_of("config").unwrap_or("default");
 
@@ -909,7 +912,7 @@ fn edit_main(config_set: ConfigSet, matches: &ArgMatches) {
         let metadata = std::fs::metadata(&config_path).expect("cannot gather file metadata");
         let last_modified = metadata.modified().expect("cannot read file last modified date");
 
-        let result = crate::edit::open_editor(&config_set, &config_path);
+        let result = crate::edit::open_editor(&config_path);
         if result {
             let new_metadata = std::fs::metadata(&config_path).expect("cannot gather file metadata");
             let new_last_modified = new_metadata.modified().expect("cannot read file last modified date");
@@ -925,7 +928,7 @@ fn edit_main(config_set: ConfigSet, matches: &ArgMatches) {
             false
         }
     }else{
-        let result = crate::edit::open_editor(&config_set, &config_path);
+        let result = crate::edit::open_editor(&config_path);
         if result {
             // If the file has been created, we should reload the espanso config
             if config_path.exists() {
@@ -941,6 +944,13 @@ fn edit_main(config_set: ConfigSet, matches: &ArgMatches) {
     };
 
     if should_reload {
+        // Load the configuration
+        let mut config_set = ConfigSet::load_default().unwrap_or_else(|e| {
+            eprintln!("{}", e);
+            eprintln!("Unable to reload espanso due to previous configuration error.");
+            exit(1);
+        });
+
         restart_main(config_set)
     }
 }