Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show a warning when undoing an undo operation #5580

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

### New features

* `jj undo` now shows a hint when undoing an undo operation that the user may
be looking for `jj restore` instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jj restore restores filesets from a revision. It is probably jj op restore instead that we want to suggest here, which restores the repo to an earlier state.


* `jj git {push,clone,fetch}` can now spawn an external `git` subprocess, via
the `git.subprocess = true` config knob. This provides an alternative that,
when turned on, fixes SSH bugs when interacting with Git remotes due to
Expand Down
14 changes: 14 additions & 0 deletions cli/src/commands/operation/undo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,20 @@ pub fn cmd_op_undo(
let template = tx.base_workspace_helper().operation_summary_template();
template.format(&bad_op, formatter.as_mut())?;
writeln!(formatter)?;

let was_undo_op = *tx.repo().view() == parent_op.view()?;
if args.operation == "@" && was_undo_op {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a review to discuss the op undo vs undo question, but I think at this point we're fairly decided: op undo and undo now behave identically, and both will display the hint.

The hint will still not be displayed if the user attempts to undo a specific operation (e.g. jj [op] undo 1234abc), with the assumption that they know what they're doing in that case.

writeln!(
ui.hint_default(),
"This action reverted an 'undo' operation. The repository is now in the same \
state as it was before the original 'undo'."
)?;
writeln!(
ui.hint_default(),
"If your goal is to undo multiple operations, consider using `jj op log` to see \
past states, and `jj op restore` to restore one of these states."
)?;
}
}
tx.finish(ui, format!("undo operation {}", bad_op.id().hex()))?;

Expand Down
38 changes: 38 additions & 0 deletions cli/tests/test_undo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,44 @@ fn test_bookmark_track_untrack_undo() {
"###);
}

#[test]
fn test_shows_a_warning_when_undoing_an_undo_operation_as_bare_jj_undo() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");

test_env.jj_cmd_ok(&repo_path, &["new"]);
test_env.jj_cmd_ok(&repo_path, &["undo"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["undo"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r"
Undid operation: 2d5b73a97567 (2001-02-03 08:05:09) undo operation 289cb69a8458456474a77cc432e8009b99f039cdcaf19ba4526753e97d70fee3fd0f410ff2b7c1d10cf0c2501702e7a85d58f9d813cdca567c377431ec4d2b97
Hint: This action reverted an 'undo' operation. The repository is now in the same state as it was before the original 'undo'.
Hint: If your goal is to undo multiple operations, consider using `jj op log` to see past states, and `jj op restore` to restore one of these states.
Working copy now at: rlvkpnrz 65b6b74e (empty) (no description set)
Parent commit : qpvuntsm 230dd059 (empty) (no description set)
");
}

#[test]
fn test_shows_no_warning_when_undoing_a_specific_undo_change() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");

test_env.jj_cmd_ok(&repo_path, &["new"]);
test_env.jj_cmd_ok(&repo_path, &["undo"]);
let stdout = test_env.jj_cmd_success(&repo_path, &["op", "log"]);
let op_id_hex = stdout[3..15].to_string();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["undo", &op_id_hex]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r"
Undid operation: 2d5b73a97567 (2001-02-03 08:05:09) undo operation 289cb69a8458456474a77cc432e8009b99f039cdcaf19ba4526753e97d70fee3fd0f410ff2b7c1d10cf0c2501702e7a85d58f9d813cdca567c377431ec4d2b97
Working copy now at: rlvkpnrz 65b6b74e (empty) (no description set)
Parent commit : qpvuntsm 230dd059 (empty) (no description set)
");
}

fn get_bookmark_output(test_env: &TestEnvironment, repo_path: &Path) -> String {
// --quiet to suppress deleted bookmarks hint
test_env.jj_cmd_success(repo_path, &["bookmark", "list", "--all-remotes", "--quiet"])
Expand Down