Skip to content

Commit d5d2164

Browse files
committed
cli split: Add a config option controlling how bookmarks move during splits
Currently, `jj split` moves bookmarks from the target revision to the second revision created by the split. Since the first revision inherits the change id of the target revision, moving the bookmarks to the first revision is less surprising (i.e. the bookmarks stay with the change id). This no-implicit-move behavior also aligns with how `jj abandon` drops bookmarks instead of moving them to the parent revision. Two releases from now, `jj split` will no longer move bookmarks to the second revision created by the split. Instead, local bookmarks associated with the target revision will move to the first revision created by the split (which inherits the target revision's change id). You can opt out of this change by setting `split.legacy-bookmark-behavior = true`, but this will likely be removed in a future release. You can also try the new behavior now by setting `split.legacy-bookmark-behavior = false`. Users who have not opted into the new behavior via the config setting will see a warning when they run `jj split` informing them about the change. The default behavior be changed in the future. The `jj split` tests for bookmarks are updated to run in all three configurations: - Config setting enabled - Config setting disabled - Config setting unset #3419
1 parent ae59bd8 commit d5d2164

File tree

4 files changed

+202
-32
lines changed

4 files changed

+202
-32
lines changed

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,16 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
2727
a warning if the user does not specify target revision explicitly. In the near
2828
future those commands will fail if target revision is not specified.
2929

30+
* In the next release, `jj split` will no longer move bookmarks to the second
31+
revision created by the split. Instead, local bookmarks associated with the
32+
target revision will move to the first revision created by the split (which
33+
inherits the target revision's change id). You can opt out of this change by
34+
setting `split.legacy-bookmark-behavior = true`, but this will likely be
35+
removed in a future release. You can also try the new behavior now by setting
36+
`split.legacy-bookmark-behavior = false`.
37+
[#3419](https://github.com/jj-vcs/jj/issues/3419)
38+
39+
3040
### New features
3141

3242
* `jj undo` now shows a hint when undoing an undo operation that the user may

cli/src/commands/split.rs

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use std::io::Write;
1515

1616
use clap_complete::ArgValueCandidates;
1717
use clap_complete::ArgValueCompleter;
18+
use jj_lib::config::ConfigGetResultExt;
1819
use jj_lib::object_id::ObjectId;
1920
use jj_lib::repo::Repo;
2021
use tracing::instrument;
@@ -196,26 +197,37 @@ The remainder will be in the second commit.
196197
commit_builder.write(tx.repo_mut())?
197198
};
198199

199-
// Mark the commit being split as rewritten to the second commit. As a
200-
// result, if @ points to the commit being split, it will point to the
201-
// second commit after the command finishes. This also means that any
202-
// bookmarks pointing to the commit being split are moved to the second
203-
// commit.
204-
tx.repo_mut()
205-
.set_rewritten_commit(commit.id().clone(), second_commit.id().clone());
200+
let legacy_bookmark_behavior = read_legacy_bookmark_behavior_setting(tx.settings(), ui)?;
201+
if legacy_bookmark_behavior {
202+
// Mark the commit being split as rewritten to the second commit. This
203+
// moves any bookmarks pointing to the target commit to the second
204+
// commit.
205+
tx.repo_mut()
206+
.set_rewritten_commit(commit.id().clone(), second_commit.id().clone());
207+
}
206208
let mut num_rebased = 0;
207209
tx.repo_mut()
208210
.transform_descendants(vec![commit.id().clone()], |mut rewriter| {
209211
num_rebased += 1;
210-
if args.parallel {
212+
if args.parallel && legacy_bookmark_behavior {
213+
// The old_parent is the second commit due to the rewrite above.
211214
rewriter
212215
.replace_parent(second_commit.id(), [first_commit.id(), second_commit.id()]);
216+
} else if args.parallel {
217+
rewriter.replace_parent(first_commit.id(), [first_commit.id(), second_commit.id()]);
218+
} else {
219+
rewriter.replace_parent(first_commit.id(), [second_commit.id()]);
213220
}
214-
// We don't need to do anything special for the non-parallel case
215-
// since we already marked the original commit as rewritten.
216221
rewriter.rebase()?.write()?;
217222
Ok(())
218223
})?;
224+
// Move the working copy commit (@) to the second commit for any workspaces
225+
// where the target commit is the working copy commit.
226+
for (workspace_id, working_copy_commit) in tx.base_repo().clone().view().wc_commit_ids() {
227+
if working_copy_commit == commit.id() {
228+
tx.repo_mut().edit(workspace_id.clone(), &second_commit)?;
229+
}
230+
}
219231

220232
if let Some(mut formatter) = ui.status_formatter() {
221233
if num_rebased > 0 {
@@ -230,3 +242,36 @@ The remainder will be in the second commit.
230242
tx.finish(ui, format!("split commit {}", commit.id().hex()))?;
231243
Ok(())
232244
}
245+
246+
/// Reads 'split.legacy-bookmark-behavior' from settings and prints a warning if
247+
/// the value is unset to alert the user to the behavior change.
248+
fn read_legacy_bookmark_behavior_setting(
249+
settings: &jj_lib::settings::UserSettings,
250+
ui: &Ui,
251+
) -> Result<bool, CommandError> {
252+
match settings
253+
.get_bool("split.legacy-bookmark-behavior")
254+
.optional()?
255+
{
256+
// Use the new behavior.
257+
Some(false) => Ok(false),
258+
// Use the legacy behavior.
259+
Some(true) | None => {
260+
writeln!(
261+
ui.warning_default(),
262+
"`jj split` will leave bookmarks on the first commit in the next release."
263+
)?;
264+
writeln!(
265+
ui.warning_default(),
266+
"Run `jj config set --repo split.legacy-bookmark-behavior false` to silence this \
267+
message and use the new behavior."
268+
)?;
269+
writeln!(
270+
ui.warning_default(),
271+
"See https://github.com/jj-vcs/jj/issues/3419"
272+
)?;
273+
// TODO: https://github.com/jj-vcs/jj/issues/3419 - Return false by default in v0.28.
274+
Ok(true)
275+
}
276+
}
277+
}

cli/src/config-schema.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,17 @@
684684
"description": "Settings for tools run by jj fix"
685685
}
686686
}
687+
},
688+
"split": {
689+
"type": "object",
690+
"description": "Settings for jj split",
691+
"properties": {
692+
"legacy-bookmark-behavior": {
693+
"type": "boolean",
694+
"description": "If true, bookmarks will move to the second commit instead of the first.",
695+
"default": false
696+
}
697+
}
687698
}
688699
}
689700
}

0 commit comments

Comments
 (0)