cli config edit: Rollback to previous config when invalid TOML is saved

CHANGELOG.md

@@ -67,6 +67,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * Commit objects in templates now have `trailers() -> List<Trailer>`, the Trailer
   objects have `key() -> String` and `value() -> String`.
 
+* `jj config edit` will now roll back to previous version if a syntax error has been introduced in the new config.
+
+
 ### Fixed bugs
 
 * Fixed crash on change-delete conflict resolution.

cli/src/commands/config/edit.rs

@@ -12,10 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use jj_lib::config::ConfigLayer;
 use tracing::instrument;
 
 use super::ConfigLevelArgs;
 use crate::cli_util::CommandHelper;
+use crate::command_error::print_error_sources;
 use crate::command_error::CommandError;
 use crate::ui::Ui;
 
@@ -40,6 +42,34 @@ pub fn cmd_config_edit(
     if !file.path().exists() {
         file.save()?;
     }
-    editor.edit_file(file.path())?;
+
+    // Editing again and again until either of these conditions is met
+    // 1. The config is OK
+    // 2. The user restores previous one
+    loop {
+        editor.edit_file(file.path())?;
+
+        // Trying to load back config. If error, prompt to continue editing
+        if let Err(e) = ConfigLayer::load_from_file(file.layer().source, file.path().to_path_buf())
+        {
+            writeln!(
+                ui.warning_default(),
+                "An error has been found inside the config:"
+            )?;
+            print_error_sources(ui, Some(&e))?;
+            let continue_editing = ui.prompt_yes_no(
+                "Do you want to keep editing the file? If not, previous config will be restored.",
+                Some(true),
+            )?;
+            if !continue_editing {
+                // Saving back previous config
+                file.save()?;
+                break;
+            }
+        } else {
+            // config is OK
+            break;
+        }
+    }
     Ok(())
 }

cli/tests/test_config_command.rs

@@ -19,6 +19,7 @@ use indoc::indoc;
 use regex::Regex;
 
 use crate::common::fake_editor_path;
+use crate::common::force_interactive;
 use crate::common::to_toml_value;
 use crate::common::TestEnvironment;
 
@@ -950,6 +951,73 @@ fn test_config_edit_repo() {
     assert!(repo_config_path.exists(), "new file should be created");
 }
 
+#[test]
+fn test_config_edit_invalid_config() {
+    let mut test_env = TestEnvironment::default();
+    let edit_script = test_env.set_up_fake_editor();
+
+    // Test re-edit
+    std::fs::write(
+        &edit_script,
+        "write\ninvalid config here\0next invocation\n\0write\ntest=\"success\"",
+    )
+    .unwrap();
+    test_env.run_jj_in(".", ["git", "init", "repo"]).success();
+    let work_dir = test_env.work_dir("repo");
+    let output = work_dir.run_jj_with(|cmd| {
+        force_interactive(cmd)
+            .args(["config", "edit", "--repo"])
+            .write_stdin("Y\n")
+    });
+    insta::assert_snapshot!(output, @r"
+    ------- stderr -------
+    Warning: An error has been found inside the config:
+    Caused by:
+    1: Configuration cannot be parsed as TOML document
+    2: TOML parse error at line 1, column 9
+      |
+    1 | invalid config here
+      |         ^
+    expected `.`, `=`
+
+    Do you want to keep editing the file? If not, previous config will be restored. (Yn): [EOF]
+    ");
+
+    let output = work_dir.run_jj(["config", "get", "test"]);
+    insta::assert_snapshot!(output, @r"
+    success
+    [EOF]"
+    );
+
+    // Test the restore previous config
+    std::fs::write(&edit_script, "write\ninvalid config here").unwrap();
+    let work_dir = test_env.work_dir("repo");
+    let output = work_dir.run_jj_with(|cmd| {
+        force_interactive(cmd)
+            .args(["config", "edit", "--repo"])
+            .write_stdin("n\n")
+    });
+    insta::assert_snapshot!(output, @r"
+    ------- stderr -------
+    Warning: An error has been found inside the config:
+    Caused by:
+    1: Configuration cannot be parsed as TOML document
+    2: TOML parse error at line 1, column 9
+      |
+    1 | invalid config here
+      |         ^
+    expected `.`, `=`
+
+    Do you want to keep editing the file? If not, previous config will be restored. (Yn): [EOF]
+    ");
+
+    let output = work_dir.run_jj(["config", "get", "test"]);
+    insta::assert_snapshot!(output, @r"
+    success
+    [EOF]"
+    );
+}
+
 #[test]
 fn test_config_path() {
     let mut test_env = TestEnvironment::default();