fix: Do not panic in config if editor not found (#3766)

* fix: Do not panic in config if editor not found

* Add tests for edit_configuration

Adds tests for no-panic condition on editor by adding an override to
edit_configuration.

* Sorry clippy :(

src/configure.rs

@@ -266,12 +266,24 @@ pub fn write_configuration(doc: &Document) {
         .expect("Error writing starship config");
 }
 
-pub fn edit_configuration() {
+pub fn edit_configuration(editor_override: Option<&str>) -> Result<(), Box<dyn std::error::Error>> {
+    // Argument currently only used for testing, but could be used to specify
+    // an editor override on the command line.
     let config_path = get_config_path();
-    let editor_cmd = shell_words::split(&get_editor()).expect("Unmatched quotes found in $EDITOR.");
 
-    let command = utils::create_command(&editor_cmd[0])
+    let editor_cmd = shell_words::split(&get_editor(editor_override))?;
-        .expect("Unable to locate editor in $PATH.")
+    let mut command = match utils::create_command(&editor_cmd[0]) {
+        Ok(cmd) => cmd,
+        Err(e) => {
+            eprintln!(
+                "Unable to find editor {:?}. Are $VISUAL and $EDITOR set correctly?",
+                editor_cmd[0]
+            );
+            return Err(Box::new(e));
+        }
+    };
+
+    let res = command
         .stdin(Stdio::inherit())
         .stdout(Stdio::inherit())
         .stderr(Stdio::inherit())
@@ -279,24 +291,20 @@ pub fn edit_configuration() {
         .arg(config_path)
         .status();
 
-    match command {
+    if let Err(e) = res {
-        Ok(_) => (),
+        eprintln!("Unable to launch editor {:?}", editor_cmd);
-        Err(error) => match error.kind() {
+        return Err(Box::new(e));
-            ErrorKind::NotFound => {
+    }
-                eprintln!(
+
-                    "Error: editor {:?} was not found. Did you set your $EDITOR or $VISUAL \
+    Ok(())
-                    environment variables correctly?",
-                    editor_cmd
-                );
-                std::process::exit(1)
-            }
-            other_error => panic!("failed to open file: {:?}", other_error),
-        },
-    };
 }
 
-fn get_editor() -> String {
+fn get_editor(editor_override: Option<&str>) -> String {
-    get_editor_internal(env::var("VISUAL").ok(), env::var("EDITOR").ok())
+    if let Some(cmd) = editor_override {
+        cmd.to_string()
+    } else {
+        get_editor_internal(std::env::var("VISUAL").ok(), std::env::var("EDITOR").ok())
+    }
 }
 
 fn get_editor_internal(visual: Option<String>, editor: Option<String>) -> String {
@@ -375,6 +383,18 @@ mod tests {
         assert_eq!(STD_EDITOR, actual);
     }
 
+    #[test]
+    fn no_panic_when_editor_unparseable() {
+        let outcome = edit_configuration(Some("\"vim"));
+        assert!(outcome.is_err());
+    }
+
+    #[test]
+    fn no_panic_when_editor_not_found() {
+        let outcome = edit_configuration(Some("this_editor_does_not_exist"));
+        assert!(outcome.is_err());
+    }
+
     #[test]
     fn test_extract_toml_paths() {
         let config = toml::toml! {

src/main.rs

@@ -187,8 +187,9 @@ fn main() {
                 if let Some(value) = value {
                     configure::update_configuration(&name, &value)
                 }
-            } else {
+            } else if let Err(reason) = configure::edit_configuration(None) {
-                configure::edit_configuration()
+                eprintln!("Could not edit configuration: {}", reason);
+                std::process::exit(1);
             }
         }
         Commands::PrintConfig { default, name } => configure::print_configuration(default, &name),