Skip to content

Commit

Permalink
cli fix: change default from -s @ to -s 'reachable(@, mutable())'
Browse files Browse the repository at this point in the history
Most of the value of `jj fix` over a shell script is in formatting commits
other than `@`. `@::` often doesn't contain those other commits, so `-s @` is a
bad default.

We could get the same effect from `-s 'mutable() & ::@'`, but `reachable()` is
a bit more explicit and simple to read.

We could also base this on excluding `trunk()`, but that just seems like an
indirection for `mutable()` that might ignore the user's intent if they have
configured part of trunk to be mutable.
  • Loading branch information
hooper committed Jun 13, 2024
1 parent faf9a9d commit 986630b
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 20 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Dropped support for automatic upgrade of repo formats used by versions before
0.12.0.

* `jj fix` now defaults to the broader revset `-s reachable(@, mutable())`
instead of `-s @`.

### Deprecations

### New features
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ use crate::ui::Ui;
pub(crate) struct FixArgs {
/// Fix files in the specified revision(s) and their descendants. If no
/// revisions are specified, this defaults to the `revsets.fix` setting, or
/// `@` if it is not set.
/// `reachable(@, mutable())` if it is not set.
#[arg(long, short)]
source: Vec<RevisionArg>,
/// Fix only these paths
Expand Down
2 changes: 1 addition & 1 deletion cli/src/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@
"fix": {
"type": "string",
"description": "Default set of revisions to fix when no explicit revset is given for jj fix",
"default": "@"
"default": "reachable(@, mutable())"
},
"log": {
"type": "string",
Expand Down
2 changes: 1 addition & 1 deletion cli/src/config/revsets.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# adding/updating any of these aliases

[revsets]
fix = "@"
fix = "reachable(@, mutable())"
log = "@ | ancestors(immutable_heads().., 2) | trunk()"

[revset-aliases]
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ And then run the command `jj fix -s @`.
###### **Options:**
* `-s`, `--source <SOURCE>` — Fix files in the specified revision(s) and their descendants. If no revisions are specified, this defaults to the `revsets.fix` setting, or `@` if it is not set
* `-s`, `--source <SOURCE>` — Fix files in the specified revision(s) and their descendants. If no revisions are specified, this defaults to the `revsets.fix` setting, or `reachable(@, mutable())` if it is not set
Expand Down
51 changes: 35 additions & 16 deletions cli/tests/test_fix_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,30 +142,49 @@ fn test_fix_sibling_commit() {
#[test]
fn test_default_revset() {
let (test_env, repo_path) = init_with_fake_formatter(&["--uppercase"]);
std::fs::write(repo_path.join("file"), "trunk1").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "trunk1"]);
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file"), "trunk2").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "trunk2"]);
test_env.jj_cmd_ok(&repo_path, &["new", "trunk1"]);
std::fs::write(repo_path.join("file"), "foo").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "foo"]);
test_env.jj_cmd_ok(&repo_path, &["new", "trunk1"]);
std::fs::write(repo_path.join("file"), "bar1").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "bar1"]);
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file"), "bar").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "bar"]);
test_env.jj_cmd_ok(&repo_path, &["new", "foo"]);
std::fs::write(repo_path.join("file"), "baz").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "baz"]);

// With no args and no revset configuration, we fix `@`.
std::fs::write(repo_path.join("file"), "bar2").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "bar2"]);
test_env.jj_cmd_ok(&repo_path, &["new"]);
std::fs::write(repo_path.join("file"), "bar3").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "bar3"]);
test_env.jj_cmd_ok(&repo_path, &["edit", "bar2"]);

// With no args and no revset configuration, we fix `reachable(@, mutable())`,
// which includes bar{1,2,3} and excludes trunk{1,2} (which is immutable) and
// foo (which is mutable but not reachable).
test_env.add_config(r#"revset-aliases."immutable_heads()" = "trunk2""#);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["fix"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Fixed 1 commits of 1 checked.
Working copy now at: mzvwutvl 5205c5f1 baz | (no description set)
Parent commit : qpvuntsm 34af74d0 foo | (no description set)
Fixed 3 commits of 3 checked.
Working copy now at: yostqsxw 0bd830d2 bar2 | (no description set)
Parent commit : yqosqzyt 4747dd17 bar1 | (no description set)
Added 0 files, modified 1 files, removed 0 files
"###);
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "trunk1"]);
insta::assert_snapshot!(content, @"trunk1");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "trunk2"]);
insta::assert_snapshot!(content, @"trunk2");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "foo"]);
insta::assert_snapshot!(content, @"foo");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "bar"]);
insta::assert_snapshot!(content, @"bar");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "baz"]);
insta::assert_snapshot!(content, @"BAZ");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "bar1"]);
insta::assert_snapshot!(content, @"BAR1");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "bar2"]);
insta::assert_snapshot!(content, @"BAR2");
let content = test_env.jj_cmd_success(&repo_path, &["print", "file", "-r", "bar3"]);
insta::assert_snapshot!(content, @"BAR3");
}

#[test]
Expand All @@ -178,8 +197,8 @@ fn test_custom_default_revset() {
std::fs::write(repo_path.join("file"), "bar").unwrap();
test_env.jj_cmd_ok(&repo_path, &["branch", "create", "bar"]);

// Check out a different commit so that the schema default `@` would behave
// differently from our customized default.
// Check out a different commit so that the schema default `reachable(@,
// mutable())` would behave differently from our customized default.
test_env.jj_cmd_ok(&repo_path, &["new", "-r", "foo"]);
test_env.add_config(r#"revsets.fix = "bar""#);

Expand Down

0 comments on commit 986630b

Please sign in to comment.