diff --git a/CHANGELOG.md b/CHANGELOG.md index e71a32f05e3..f77dda82e19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). back to prompting the user if the heuristics are inconclusive. It can also run in non-interactive mode, which aborts if prompting would be needed. +* `jj git push` can now be configured to ask for confirmation before pushing + changes to a remote using `git.confirm-before-push`. + ### Fixed bugs * `jj arrange` now scrolls the viewport to keep the selected commit visible diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index ef6de4769ef..afc226139c9 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -535,6 +535,13 @@ pub async fn cmd_git_push( return Ok(()); } + let needs_confirm = !args.dry_run + && match tx.settings().get("git.confirm-before-push")? { + PushConfirmChoice::Always => true, + PushConfirmChoice::Never => false, + PushConfirmChoice::Auto => ref_updates.bookmarks.len() + ref_updates.tags.len() > 1, + }; + if !args.dry_run && tx.settings().get_bool("git.sign-on-push")? { let to_push_expr = ready_to_push_revset_expression(&tx, remote, &ref_updates); ref_updates = sign_commits_before_push(ui, &mut tx, to_push_expr, ref_updates).await?; @@ -554,6 +561,11 @@ pub async fn cmd_git_push( return Ok(()); } + if needs_confirm && !ui.prompt_yes_no("Continue?", Some(true))? { + writeln!(ui.status(), "Aborting; nothing was changed.")?; + return Ok(()); + } + let git_settings = GitSettings::from_settings(tx.settings())?; let options = GitPushOptions { remote_push_options: args.option.clone(), @@ -601,6 +613,21 @@ pub async fn cmd_git_push( } } +#[derive(Clone, Copy, PartialEq, Eq, Debug, serde::Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum PushConfirmChoice { + /// Always ask for confirmation before pushing + Always, + /// Never prompt the user before pushing a change + Never, + /// Only prompt if more than one bookmark/tag is about to be pushed + /// + /// If more than one bookmark or tag is moved in the same push, it is + /// possible that some of them were unintentional, so we should give the + /// user a chance to correct their mistake. + Auto, +} + #[derive(Clone, Debug)] struct RejectedCommitReason { commit: Commit, diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index aa841243910..30727abb25d 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -559,6 +559,11 @@ "enum": ["sha1", "sha256"], "description": "Object hash algorithm used when initializing a new Git repository", "default": "sha1" + }, + "confirm-before-push": { + "enum": ["always", "never", "auto"], + "description": "When to prompt user before pushing changes to remote", + "default": "never" } } }, diff --git a/cli/src/config/misc.toml b/cli/src/config/misc.toml index 70ed210c17e..460128d2da9 100644 --- a/cli/src/config/misc.toml +++ b/cli/src/config/misc.toml @@ -29,6 +29,7 @@ colocate = true object-hash = "sha1" private-commits = "none()" sign-on-push = false +confirm-before-push = "never" track-default-bookmark-on-clone = true [ui] diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index 35de0bee209..7ff8afab095 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -19,6 +19,7 @@ use testutils::git; use crate::common::CommandOutput; use crate::common::TestEnvironment; use crate::common::TestWorkDir; +use crate::common::force_interactive; use crate::common::to_toml_value; fn git_repo_dir_for_jj_repo(work_dir: &TestWorkDir<'_>) -> std::path::PathBuf { @@ -145,6 +146,85 @@ fn test_git_push_default_remote_selection() { "); } +#[test] +fn test_git_push_confirm() { + let test_env = TestEnvironment::default(); + set_up(&test_env); + test_env.add_config("remotes.origin.auto-track-bookmarks = '*'"); + test_env.add_config("git.confirm-before-push = 'always'"); + let work_dir = test_env.work_dir("local"); + test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#); + // Update some bookmarks. `bookmark1` is not a current bookmark, but + // `bookmark2` and `my-bookmark` are. + work_dir + .run_jj(["describe", "bookmark1", "-m", "modified bookmark1 commit"]) + .success(); + work_dir.run_jj(["new", "bookmark2"]).success(); + work_dir + .run_jj(["bookmark", "set", "bookmark2", "-r@"]) + .success(); + work_dir + .run_jj(["bookmark", "create", "-r@", "my-bookmark"]) + .success(); + work_dir.run_jj(["describe", "-m", "foo"]).success(); + // Check the setup + insta::assert_snapshot!(get_bookmark_output(&work_dir), @" + bookmark1: qpvuntsm e5ce6d9a (empty) modified bookmark1 commit + @origin (ahead by 1 commits, behind by 1 commits): qpvuntsm/1 9b2e76de (hidden) (empty) description 1 + bookmark2: yostqsxw 88ca14a7 (empty) foo + @origin (behind by 1 commits): zsuskuln 38a20473 (empty) description 2 + my-bookmark: yostqsxw 88ca14a7 (empty) foo + @origin (not created yet) + [EOF] + "); + // Abort push, should not change anything + let output = work_dir.run_jj_with(|cmd| { + force_interactive(cmd) + .args(["git", "push"]) + .write_stdin("n\n") + }); + insta::assert_snapshot!(output, @" + ------- stderr ------- + Changes to push to origin: + bookmark: bookmark2 [move forward from 38a204733702 to 88ca14a7d46f] + bookmark: my-bookmark [add to 88ca14a7d46f] + Continue? (Yn): Aborting; nothing was changed. + [EOF] + "); + // Make sure nothing has changed + insta::assert_snapshot!(get_bookmark_output(&work_dir), @" + bookmark1: qpvuntsm e5ce6d9a (empty) modified bookmark1 commit + @origin (ahead by 1 commits, behind by 1 commits): qpvuntsm/1 9b2e76de (hidden) (empty) description 1 + bookmark2: yostqsxw 88ca14a7 (empty) foo + @origin (behind by 1 commits): zsuskuln 38a20473 (empty) description 2 + my-bookmark: yostqsxw 88ca14a7 (empty) foo + @origin (not created yet) + [EOF] + "); + // Accept push, should go though to remote + let output = work_dir.run_jj_with(|cmd| { + force_interactive(cmd) + .args(["git", "push"]) + .write_stdin("y\n") + }); + insta::assert_snapshot!(output, @" + ------- stderr ------- + Changes to push to origin: + bookmark: bookmark2 [move forward from 38a204733702 to 88ca14a7d46f] + bookmark: my-bookmark [add to 88ca14a7d46f] + Continue? (Yn): [EOF] + "); + insta::assert_snapshot!(get_bookmark_output(&work_dir), @" + bookmark1: qpvuntsm e5ce6d9a (empty) modified bookmark1 commit + @origin (ahead by 1 commits, behind by 1 commits): qpvuntsm/1 9b2e76de (hidden) (empty) description 1 + bookmark2: yostqsxw 88ca14a7 (empty) foo + @origin: yostqsxw 88ca14a7 (empty) foo + my-bookmark: yostqsxw 88ca14a7 (empty) foo + @origin: yostqsxw 88ca14a7 (empty) foo + [EOF] + "); +} + #[test] fn test_git_push_current_bookmark() { let test_env = TestEnvironment::default(); diff --git a/docs/config.md b/docs/config.md index 2a8cf2beb68..436a86e66e1 100644 --- a/docs/config.md +++ b/docs/config.md @@ -1967,6 +1967,19 @@ git_push_bookmark = '"martinvonz/push-" ++ change_id.short()' This template should include expressions like `change_id` to generate unique and stable bookmark. +### Ask for confirmation on push + +The command `jj git push` can be configured to ask for confirmation before +applying any changes to a remote using the setting `git.confirm-before-push`. +Possible values are `always`, `never` and `auto` (default: `never`); a value +of `auto` will prompt the user only when multiple bookmarks or tags are about +to be updated in a single push. For example: + +```toml +[git] +confirm-before-push = 'auto' +``` + ### Set of private commits You can configure the set of private commits by setting `git.private-commits` to