ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

Rename conflict and file revsets to conflicts and files.

Browse source 
See discussion thread in linked issue.

With this PR, all revset functions in [BUILTIN_FUNCTION_MAP](8d166c7642/lib/src/revset.rs (L570))
that return multiple values are either named in plural or the naming is hard to misunderstand (e.g. `reachable`)

Fixes: #4122
This commit is contained in:
Essien Ita Essien 2024-08-24 18:53:00 +01:00 committed by Essien Ita Essien
parent cf7847d784
commit 895d53f395
6 changed files with 66 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -94,6 +94,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
commit to commit. It now either follows the flags `--edit|--no-edit` or it
gets the mode from `ui.movement.edit`.
* `conflict` revset has been renamed to `conflicts`. With this, all revsets that
could potentially return multiple results are named in the plural.
### Deprecations
* `jj untrack` has been renamed to `jj file untrack`.

4
cli/tests/test_log_command.rs
View file

@ -873,7 +873,7 @@ fn test_log_filtered_by_path() {
~
"###);
// file() revset doesn't filter the diff.
// files() revset doesn't filter the diff.
let stdout = test_env.jj_cmd_success(
&repo_path,
&[
@ -881,7 +881,7 @@ fn test_log_filtered_by_path() {
"-T",
"description",
"-s",
"-rfile(file2)",
"-rfiles(file2)",
"--no-graph",
],
);

50
cli/tests/test_revset_output.rs
View file

@ -129,24 +129,24 @@ fn test_bad_function_call() {
= Expected expression of type integer
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file()"]);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "files()"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: Function "file": Expected at least 1 arguments
Caused by: --> 1:6
Error: Failed to parse revset: Function "files": Expected at least 1 arguments
Caused by: --> 1:7
|
1 | file()
1 | files()
| ^
|
= Function "file": Expected at least 1 arguments
= Function "files": Expected at least 1 arguments
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "file(not::a-fileset)"]);
insta::assert_snapshot!(stderr, @r#"
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "files(not::a-fileset)"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: In fileset expression
Caused by:
1: --> 1:6
1: --> 1:7
|
1 | file(not::a-fileset)
1 | files(not::a-fileset)
| ^------------^
|
= In fileset expression
@ -156,15 +156,15 @@ fn test_bad_function_call() {
| ^---
|
= expected <identifier>, <string_literal>, or <raw_string_literal>
"#);
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", r#"file(foo:"bar")"#]);
insta::assert_snapshot!(stderr, @r#"
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", r#"files(foo:"bar")"#]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: In fileset expression
Caused by:
1: --> 1:6
1: --> 1:7
|
1 | file(foo:"bar")
1 | files(foo:"bar")
| ^-------^
|
= In fileset expression
@ -175,15 +175,15 @@ fn test_bad_function_call() {
|
= Invalid file pattern
3: Invalid file pattern kind "foo:"
"#);
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", r#"file(a, "../out")"#]);
insta::assert_snapshot!(stderr.replace('\\', "/"), @r#"
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", r#"files(a, "../out")"#]);
insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
Error: Failed to parse revset: In fileset expression
Caused by:
1: --> 1:9
1: --> 1:10
|
1 | file(a, "../out")
1 | files(a, "../out")
| ^------^
|
= In fileset expression
@ -195,7 +195,7 @@ fn test_bad_function_call() {
= Invalid file pattern
3: Path "../out" is not in the repo "."
4: Invalid component ".." in repo-relative path "../out"
"#);
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "bookmarks(bad:pattern)"]);
insta::assert_snapshot!(stderr, @r###"
@ -334,17 +334,17 @@ fn test_parse_warning() {
= untracked_remote_branches() is deprecated; use untracked_remote_bookmarks() instead
"#);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r", "file(foo, bar)"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r", "files(foo, bar)"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r#"
insta::assert_snapshot!(stderr, @r###"
Warning: In revset expression
--> 1:6
--> 1:7
|
1 | file(foo, bar)
1 | files(foo, bar)
| ^------^
|
= Multi-argument patterns syntax is deprecated; separate them with |
"#);
"###);
}
#[test]

4
docs/revsets.md
View file

@ -274,7 +274,7 @@ given [string pattern](#string-patterns).
* `empty()`: Commits modifying no files. This also includes `merges()` without
user modifications and `root()`.
* `file(expression)`: Commits modifying paths matching the given [fileset
* `files(expression)`: Commits modifying paths matching the given [fileset
expression](filesets.md).
Paths are relative to the directory `jj` was invoked from. A directory name
@ -296,7 +296,7 @@ given [string pattern](#string-patterns).
For example, `diff_contains("TODO", "src")` will search revisions where "TODO"
is added to or removed from files under "src".
* `conflict()`: Commits with conflicts.
* `conflicts()`: Commits with conflicts.
* `present(x)`: Same as `x`, but evaluated to `none()` if any of the commits
in `x` doesn't exist (e.g. is an unknown bookmark name.)

22
lib/src/revset.rs
View file

@ -789,7 +789,14 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
function.expect_no_arguments()?;
Ok(RevsetExpression::is_empty())
});
map.insert("file", |diagnostics, function, context| {
map.insert("files", |diagnostics, function, context| {
// TODO: Remove in jj 0.28+
if function.name != "files" {
diagnostics.add_warning(RevsetParseError::expression(
"file() is deprecated; use files() instead",
function.name_span,
));
}
let ctx = context.workspace.as_ref().ok_or_else(|| {
RevsetParseError::with_span(
RevsetParseErrorKind::FsPathWithoutWorkspace,
@ -810,6 +817,8 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
let expr = FilesetExpression::union_all(file_expressions);
Ok(RevsetExpression::filter(RevsetFilterPredicate::File(expr)))
});
// TODO: Remove in jj 0.28+
map.insert("file", map["files"]);
map.insert("diff_contains", |diagnostics, function, context| {
let ([text_arg], [files_opt_arg]) = function.expect_arguments()?;
let text = expect_string_pattern(diagnostics, text_arg)?;
@ -830,10 +839,19 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
RevsetFilterPredicate::DiffContains { text, files },
))
});
map.insert("conflict", |_diagnostics, function, _context| {
map.insert("conflicts", |diagnostics, function, _context| {
// TODO: Remove in jj 0.28+
if function.name != "conflicts" {
diagnostics.add_warning(RevsetParseError::expression(
"file() is deprecated; use files() instead",
function.name_span,
));
}
function.expect_no_arguments()?;
Ok(RevsetExpression::filter(RevsetFilterPredicate::HasConflict))
});
// TODO: Remove in jj 0.28+
map.insert("conflict", map["conflicts"]);
map.insert("present", |diagnostics, function, context| {
let [arg] = function.expect_exact_arguments()?;
let expression = lower_expression(diagnostics, arg, context)?;

15
lib/tests/test_revset.rs
View file

@ -3171,7 +3171,7 @@ fn test_evaluate_expression_file() {
assert_eq!(
resolve_commit_ids_in_workspace(
mut_repo,
r#"file("repo/added_clean_clean")"#,
r#"files("repo/added_clean_clean")"#,
&test_workspace.workspace,
Some(test_workspace.workspace.workspace_root().parent().unwrap()),
),
@ -3180,7 +3180,7 @@ fn test_evaluate_expression_file() {
assert_eq!(
resolve_commit_ids_in_workspace(
mut_repo,
r#"file("added_clean_clean"|"added_modified_clean")"#,
r#"files("added_clean_clean"|"added_modified_clean")"#,
&test_workspace.workspace,
Some(test_workspace.workspace.workspace_root()),
),
@ -3189,7 +3189,10 @@ fn test_evaluate_expression_file() {
assert_eq!(
resolve_commit_ids_in_workspace(
mut_repo,
&format!(r#"{}:: & file("added_modified_clean")"#, commit2.id().hex()),
&format!(
r#"{}:: & files("added_modified_clean")"#,
commit2.id().hex()
),
&test_workspace.workspace,
Some(test_workspace.workspace.workspace_root()),
),
@ -3387,7 +3390,7 @@ fn test_evaluate_expression_file_merged_parents() {
};
assert_eq!(
query("file('file1')"),
query("files('file1')"),
vec![
commit4.id().clone(),
commit3.id().clone(),
@ -3396,7 +3399,7 @@ fn test_evaluate_expression_file_merged_parents() {
]
);
assert_eq!(
query("file('file2')"),
query("files('file2')"),
vec![
commit3.id().clone(),
commit2.id().clone(),
@ -3453,7 +3456,7 @@ fn test_evaluate_expression_conflict() {
// Only commit4 has a conflict
assert_eq!(
resolve_commit_ids(mut_repo, "conflict()"),
resolve_commit_ids(mut_repo, "conflicts()"),
vec![commit4.id().clone()]
);
}