bookmarks: First step to make target revision a required argument to …

…bookmark create/move/set.

With this change a warning is shown if the user does not explicitly specify the target revision, but the behavior is unchanged (it still defaults to the working copy).
In the future the warning will be turned into an error. In other words, it will be required to specify target revision.

The bulk of the changes here are to prepare tests for the upcoming change, to make the transition easier.

For additional details please see:
* #5374
* #5363

CHANGELOG.md

@@ -51,6 +51,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * `jj log` can now show cryptographic commit signatures. The output can be
   controlled by the `ui.show-cryptographic-signatures=true` config knob.
 
+* This release takes the first steps make target revision a required argument of
+  `bookmark create`, `bookmark move` and `bookmark set`. Those commands will
+  display a warning if the user does not specify target revision  explicitly. In
+  the near future those commands will fail if target revision is not specified.
+
 ### Breaking changes
 
 * `jj abandon` now deletes bookmarks pointing to the revisions to be abandoned.

cli/src/commands/bookmark/create.rs

@@ -31,7 +31,9 @@ pub struct BookmarkCreateArgs {
     /// The bookmark's target revision
     //
     // The `--to` alias exists for making it easier for the user to switch
-    // between `bookmark create`, `bookmark move`, and `bookmark set`.
+    // between `bookmark create`, `bookmark move`, and `bookmark set`. Currently target revision
+    // defaults to the working copy if not specified, but in the near future it will be required to
+    // explicitly specify it.
     #[arg(
         long, short,
         visible_alias = "to",
@@ -51,6 +53,14 @@ pub fn cmd_bookmark_create(
     args: &BookmarkCreateArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
+    // TODO: Make args.revision required in jj 0.32+. See https://github.com/jj-vcs/jj/issues/5374.
+    if args.revision.is_none() {
+        writeln!(
+            ui.warning_default(),
+            "Target revision was not specified, defaulting to the working copy (-r@). In the near \
+             future it will be required to explicitly specify target revision."
+        )?;
+    }
     let target_commit = workspace_command
         .resolve_single_rev(ui, args.revision.as_ref().unwrap_or(&RevisionArg::AT))?;
     let view = workspace_command.repo().view();
@@ -90,9 +100,6 @@ pub fn cmd_bookmark_create(
         tx.write_commit_summary(formatter.as_mut(), &target_commit)?;
         writeln!(formatter)?;
     }
-    if bookmark_names.len() > 1 && args.revision.is_none() {
-        writeln!(ui.hint_default(), "Use -r to specify the target revision.")?;
-    }
 
     tx.finish(
         ui,

cli/src/commands/bookmark/move.rs

@@ -56,14 +56,14 @@ pub struct BookmarkMoveArgs {
 
     /// Move bookmarks to this revision
     // We intentionally do not support the short `-t` for `--to` since we don't
-    // support `-f` for `--from`.
+    // support `-f` for `--from`. Currently this defaults to the working copy, but in the near
+    // future it will be required to explicitly specify it.
     #[arg(
         long,
-        default_value = "@",
         value_name = "REVSET",
         add = ArgValueCandidates::new(complete::all_revisions),
     )]
-    to: RevisionArg,
+    to: Option<RevisionArg>,
 
     /// Allow moving bookmarks backwards or sideways
     #[arg(long, short = 'B')]
@@ -91,8 +91,16 @@ pub fn cmd_bookmark_move(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     let repo = workspace_command.repo().clone();
-
-    let target_commit = workspace_command.resolve_single_rev(ui, &args.to)?;
+    // TODO: Make args.to required in jj 0.32+. See https://github.com/jj-vcs/jj/issues/5374.
+    if args.to.is_none() {
+        writeln!(
+            ui.warning_default(),
+            "Target revision was not specified, defaulting to the working copy (--to=@). In the \
+             near future it will be required to explicitly specify it."
+        )?;
+    }
+    let target_commit =
+        workspace_command.resolve_single_rev(ui, args.to.as_ref().unwrap_or(&RevisionArg::AT))?;
     let matched_bookmarks = {
         let is_source_ref: Box<dyn Fn(&RefTarget) -> _> = if !args.from.is_empty() {
             let is_source_commit = workspace_command

cli/src/commands/bookmark/set.rs

@@ -30,6 +30,9 @@ use crate::ui::Ui;
 #[derive(clap::Args, Clone, Debug)]
 pub struct BookmarkSetArgs {
     /// The bookmark's target revision
+    //
+    // Currently target revision defaults to the working copy if not specified, but in the near
+    // future it will be required to explicitly specify it.
     #[arg(
         long, short,
         visible_alias = "to",
@@ -57,6 +60,14 @@ pub fn cmd_bookmark_set(
     args: &BookmarkSetArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
+    // TODO: Make args.revision required in jj 0.32+. See https://github.com/jj-vcs/jj/issues/5374.
+    if args.revision.is_none() {
+        writeln!(
+            ui.warning_default(),
+            "Target revision was not specified, defaulting to the working copy (--revision=@). In \
+             the near future it will be required to explicitly specify target revision."
+        )?;
+    }
     let target_commit = workspace_command
         .resolve_single_rev(ui, args.revision.as_ref().unwrap_or(&RevisionArg::AT))?;
     let repo = workspace_command.repo().as_ref();

cli/tests/cli-reference@.md.snap

@@ -421,8 +421,6 @@ $ jj bookmark move --from 'heads(::@- & bookmarks())' --to @-
 
 * `--from <REVSETS>` — Move bookmarks from the given revisions
 * `--to <REVSET>` — Move bookmarks to this revision
-
-  Default value: `@`
 * `-B`, `--allow-backwards` — Allow moving bookmarks backwards or sideways
 
 

cli/tests/test_abandon_command.rs

@@ -25,7 +25,7 @@ fn create_commit(test_env: &TestEnvironment, repo_path: &Path, name: &str, paren
         test_env.jj_cmd_ok(repo_path, &args);
     }
     std::fs::write(repo_path.join(name), format!("{name}\n")).unwrap();
-    test_env.jj_cmd_ok(repo_path, &["bookmark", "create", name]);
+    test_env.jj_cmd_ok(repo_path, &["bookmark", "create", name, "-r", "@"]);
 }
 
 #[test]

cli/tests/test_advance_bookmarks.rs

@@ -114,7 +114,10 @@ fn test_advance_bookmarks_at_minus(make_commit: CommitFn) {
     let workspace_path = test_env.env_root().join("repo");
 
     set_advance_bookmarks(&test_env, true);
-    test_env.jj_cmd_ok(&workspace_path, &["bookmark", "create", "test_bookmark"]);
+    test_env.jj_cmd_ok(
+        &workspace_path,
+        &["bookmark", "create", "test_bookmark", "-r", "@"],
+    );
 
     insta::allow_duplicates! {
     insta::assert_snapshot!(get_log_output_with_bookmarks(&test_env, &workspace_path), @r###"
@@ -134,7 +137,10 @@ fn test_advance_bookmarks_at_minus(make_commit: CommitFn) {
 
     // Create a second bookmark pointing to @. On the next commit, only the first
     // bookmark, which points to @-, will advance.
-    test_env.jj_cmd_ok(&workspace_path, &["bookmark", "create", "test_bookmark2"]);
+    test_env.jj_cmd_ok(
+        &workspace_path,
+        &["bookmark", "create", "test_bookmark2", "-r", "@"],
+    );
     make_commit(&test_env, &workspace_path, "second");
     insta::allow_duplicates! {
     insta::assert_snapshot!(get_log_output_with_bookmarks(&test_env, &workspace_path), @r###"

cli/tests/test_alias.rs

@@ -25,7 +25,10 @@ fn test_alias_basic() {
     let repo_path = test_env.env_root().join("repo");
 
     test_env.add_config(r#"aliases.bk = ["log", "-r", "@", "-T", "bookmarks"]"#);
-    test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "my-bookmark"]);
+    test_env.jj_cmd_ok(
+        &repo_path,
+        &["bookmark", "create", "my-bookmark", "-r", "@"],
+    );
     let stdout = test_env.jj_cmd_success(&repo_path, &["bk"]);
     insta::assert_snapshot!(stdout, @r###"
     @  my-bookmark

cli/tests/test_backout_command.rs

@@ -33,7 +33,7 @@ fn create_commit(
     for (name, contents) in files {
         std::fs::write(repo_path.join(name), contents).unwrap();
     }
-    test_env.jj_cmd_ok(repo_path, &["bookmark", "create", name]);
+    test_env.jj_cmd_ok(repo_path, &["bookmark", "create", "-r@", name]);
 }
 
 #[test]