bookmarks: Make target revision a required argument to create/move/set

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]