From c8fe9963366788793fe1129f05083e270199e6e8 Mon Sep 17 00:00:00 2001 From: Philip Metzger Date: Sat, 7 Dec 2024 22:15:46 +0100 Subject: [PATCH 1/2] commit: Add `commit.is_hidden(repo)` To be used in the next diff. Also simplify the `hidden()` revset with it. --- cli/src/commit_templater.rs | 4 ++-- lib/src/commit.rs | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index c0746a01bf..785fe43253 100644 --- a/cli/src/commit_templater.rs +++ b/cli/src/commit_templater.rs @@ -774,8 +774,8 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm function.expect_no_arguments()?; let repo = language.repo; let out_property = self_property.map(|commit| { - let maybe_entries = repo.resolve_change_id(commit.change_id()); - maybe_entries.map_or(true, |entries| !entries.contains(commit.id())) + let value = commit.is_hidden(repo); + value.unwrap_or_default() }); Ok(L::wrap_boolean(out_property)) }, diff --git a/lib/src/commit.rs b/lib/src/commit.rs index 9d5f210b3c..14a6303c47 100644 --- a/lib/src/commit.rs +++ b/lib/src/commit.rs @@ -162,6 +162,12 @@ impl Commit { &self.data.committer } + /// A commit is hidden, if its commit id is not in the predecessor set. + pub fn is_hidden(&self, repo: &dyn Repo) -> BackendResult { + let maybe_entries = repo.resolve_change_id(self.change_id()); + Ok(maybe_entries.map_or(true, |entries| !entries.contains(&self.id))) + } + /// A commit is discardable if it has no change from its parent, and an /// empty description. pub fn is_discardable(&self, repo: &dyn Repo) -> BackendResult { From e08a04ee0fc3fad440dde94e58f2ebab958bf4f7 Mon Sep 17 00:00:00 2001 From: Philip Metzger Date: Wed, 4 Dec 2024 20:15:28 +0100 Subject: [PATCH 2/2] bookmark: Make `bookmark {create, set, move}` unhide hidden commits This was discussed in the Discord a while ago, and this is the logical and consistent conclusion. Implementing it as such makes it consistent with both `jj edit` and `jj new` which unhide any predecessor. --- cli/src/commands/bookmark/create.rs | 7 +++ cli/src/commands/bookmark/move.rs | 6 +++ cli/src/commands/bookmark/set.rs | 7 +++ cli/tests/test_bookmark_command.rs | 67 +++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+) diff --git a/cli/src/commands/bookmark/create.rs b/cli/src/commands/bookmark/create.rs index cd52bb889b..06a6cc4409 100644 --- a/cli/src/commands/bookmark/create.rs +++ b/cli/src/commands/bookmark/create.rs @@ -53,7 +53,9 @@ pub fn cmd_bookmark_create( let mut workspace_command = command.workspace_helper(ui)?; let target_commit = workspace_command .resolve_single_rev(ui, args.revision.as_ref().unwrap_or(&RevisionArg::AT))?; + let repo = workspace_command.repo(); let view = workspace_command.repo().view(); + let is_hidden = target_commit.is_hidden(repo.as_ref())?; let bookmark_names = &args.names; for name in bookmark_names { if view.get_local_bookmark(name).is_present() { @@ -94,6 +96,11 @@ pub fn cmd_bookmark_create( writeln!(ui.hint_default(), "Use -r to specify the target revision.")?; } + // The commit was hidden, so make it visible again. + if is_hidden { + tx.repo_mut().add_head(&target_commit)?; + } + tx.finish( ui, format!( diff --git a/cli/src/commands/bookmark/move.rs b/cli/src/commands/bookmark/move.rs index c6a229f51d..943235b3e8 100644 --- a/cli/src/commands/bookmark/move.rs +++ b/cli/src/commands/bookmark/move.rs @@ -91,6 +91,7 @@ pub fn cmd_bookmark_move( let repo = workspace_command.repo().clone(); let target_commit = workspace_command.resolve_single_rev(ui, &args.to)?; + let is_hidden = target_commit.is_hidden(repo.as_ref())?; let matched_bookmarks = { let is_source_ref: Box _> = if !args.from.is_empty() { let is_source_commit = workspace_command @@ -168,6 +169,11 @@ pub fn cmd_bookmark_move( )?; } + // The commit was hidden, unhide it. + if is_hidden { + tx.repo_mut().add_head(&target_commit)?; + } + tx.finish( ui, format!( diff --git a/cli/src/commands/bookmark/set.rs b/cli/src/commands/bookmark/set.rs index cb1af9cbdf..f00e3ecc9d 100644 --- a/cli/src/commands/bookmark/set.rs +++ b/cli/src/commands/bookmark/set.rs @@ -61,6 +61,7 @@ pub fn cmd_bookmark_set( .resolve_single_rev(ui, args.revision.as_ref().unwrap_or(&RevisionArg::AT))?; let repo = workspace_command.repo().as_ref(); let bookmark_names = &args.names; + let is_hidden = target_commit.is_hidden(repo)?; let mut new_bookmark_count = 0; let mut moved_bookmark_count = 0; for name in bookmark_names { @@ -106,6 +107,12 @@ pub fn cmd_bookmark_set( if bookmark_names.len() > 1 && args.revision.is_none() { writeln!(ui.hint_default(), "Use -r to specify the target revision.")?; } + + // The commit is hidden, so unhide it. + if is_hidden { + tx.repo_mut().add_head(&target_commit)?; + } + if new_bookmark_count > 0 { // TODO: delete this hint in jj 0.25+ writeln!( diff --git a/cli/tests/test_bookmark_command.rs b/cli/tests/test_bookmark_command.rs index 6d89338258..65faae6dbc 100644 --- a/cli/tests/test_bookmark_command.rs +++ b/cli/tests/test_bookmark_command.rs @@ -1898,6 +1898,73 @@ fn test_bookmark_list_conflicted() { "###); } +#[test] +fn test_bookmark_create_onto_hidden_unhides() { + 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"); + + std::fs::write(repo_path.join("a.txt"), "AA").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "A"]); + // Emulate a simple commit change, where we want to recover the initial version. + std::fs::write(repo_path.join("b.txt"), "BB").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["debug", "snapshot"]); + std::fs::write(repo_path.join("b.txt"), "Art").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "B"]); + // Create our bookmark onto the hidden commit. + let (stdout, _) = test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "back"]); + insta::assert_snapshot!(stdout, r#""#); +} + +#[test] +fn test_bookmark_move_onto_hidden_unhides() { + 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"); + + std::fs::write(repo_path.join("a.txt"), "AA").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "A"]); + // Create our bookmark on the first commit. It will be moved to a predecessor of + // the second one. + test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "back"]); + // Emulate a simple commit change, where we want to recover the initial version. + std::fs::write(repo_path.join("b.txt"), "BB").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["debug", "snapshot"]); + std::fs::write(repo_path.join("b.txt"), "Art").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "B"]); + + insta::assert_snapshot!(get_evolog_output(&test_env, &repo_path), r#""#); + + let (stdout, _) = + test_env.jj_cmd_ok(&repo_path, &["bookmark", "move", "back", "-r", ""]); + insta::assert_snapshot!(stdout, r#""#); +} + +#[test] +fn test_bookmark_set_onto_hidden_unhides() { + // TODO: write + 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"); + + std::fs::write(repo_path.join("a.txt"), "AA").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "A"]); + // Emulate a simple commit change, where we want to recover the initial version. + std::fs::write(repo_path.join("b.txt"), "BB").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["debug", "snapshot"]); + std::fs::write(repo_path.join("b.txt"), "Art").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "B"]); + insta::assert_snapshot!(get_evolog_output(&test_env, &repo_path), r#""#); + let (stdout, _) = + test_env.jj_cmd_ok(&repo_path, &["bookmark", "set", "back", "-r", ""]); + insta::assert_snapshot!(stdout, r#""#); +} + +fn get_evolog_output(test_env: &TestEnvironment, cwd: &Path) -> String { + let template = r#"change_id ++ " " ++ commit_id"#; + test_env.jj_cmd_success(cwd, &["evolog", "-T", template]) +} + fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String { let template = r#"bookmarks ++ " " ++ commit_id.short()"#; test_env.jj_cmd_success(cwd, &["log", "-T", template])