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

@@ -22,6 +22,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Deprecations
 
+* This release takes the first steps to make target revision required in
+  `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.
+
 ### New features
 
 * `jj undo` now shows a hint when undoing an undo operation that the user may

cli/src/commands/bookmark/create.rs

@@ -28,10 +28,13 @@ use crate::ui::Ui;
 /// Create a new bookmark
 #[derive(clap::Args, Clone, Debug)]
 pub struct BookmarkCreateArgs {
+    // TODO(#5374): Make required in jj 0.32+
     /// 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 +54,13 @@ pub fn cmd_bookmark_create(
     args: &BookmarkCreateArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
+    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,10 +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,
         format!(

cli/src/commands/bookmark/move.rs

@@ -54,16 +54,17 @@ pub struct BookmarkMoveArgs {
     )]
     from: Vec<RevisionArg>,
 
+    // TODO(#5374): Make required in jj 0.32+
     /// 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 +92,15 @@ 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)?;
+    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

@@ -29,7 +29,11 @@ use crate::ui::Ui;
 /// Create or update a bookmark to point to a certain commit
 #[derive(clap::Args, Clone, Debug)]
 pub struct BookmarkSetArgs {
+    // TODO(#5374): Make required in jj 0.32+
     /// 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 +61,13 @@ pub fn cmd_bookmark_set(
     args: &BookmarkSetArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
+    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]