mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2025-01-28 15:26:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 17

diff: add merge-tools.*.diff-expected-exit-codes
Some checks failed
binaries / Build binary artifacts (push) Waiting to run
Details
website / prerelease-docs-build-deploy (ubuntu-24.04) (push) Waiting to run
Details
Scorecards supply-chain security / Scorecards analysis (push) Waiting to run
Details
nix / flake check (push) Has been cancelled
Details
build / build (, macos-13) (push) Has been cancelled
Details
build / build (, macos-14) (push) Has been cancelled
Details
build / build (, ubuntu-24.04) (push) Has been cancelled
Details
build / build (, windows-latest) (push) Has been cancelled
Details
build / build (--all-features, ubuntu-24.04) (push) Has been cancelled
Details
build / Build jj-lib without Git support (push) Has been cancelled
Details
build / Check protos (push) Has been cancelled
Details
build / Check formatting (push) Has been cancelled
Details
build / Run doctests (push) Has been cancelled
Details
build / Check that MkDocs can build the docs (push) Has been cancelled
Details
build / Check that MkDocs can build the docs with latest Python and uv (push) Has been cancelled
Details
build / cargo-deny (advisories) (push) Has been cancelled
Details
build / cargo-deny (bans licenses sources) (push) Has been cancelled
Details
build / Clippy check (push) Has been cancelled
Details
Codespell / Codespell (push) Has been cancelled
Details

Browse source 
Certain tools (`diff`, `delta`) exit with code 1 to indicate there was
a difference. This allows selectively suppressing the "Tool exited with
... status" warning from jj when generating a diff.

example:
```toml
[merge.tools.delta]
diff-expected-exit-codes = [0, 1]
```
This commit is contained in:
Bryce Berger 2025-01-03 14:12:35 -05:00
parent d001291a27
commit b1b2c62c3e
6 changed files with 117 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -48,6 +48,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
* `jj status` now shows untracked files when they reside directly under a tracked directory.
* New `merge-tools.<TOOL>.diff-expected-exit-codes` config option to suppress
warnings from tools exiting with non-zero exit codes.
### Fixed bugs
* Fixed diff selection by external tools with `jj split`/`commit -i FILESETS`.

8
cli/src/config-schema.json
View file

@ -380,6 +380,14 @@
"type": "string"
}
},
"diff-expected-exit-codes": {
"type": "array",
"items": {
"type": "integer"
},
"description": "Array of exit codes that do not indicate tool failure, i.e. [0, 1] for unix diff.",
"default": [0]
},
"diff-invocation-mode": {
"description": "Invoke the tool with directories or individual files",
"enum": [

8
cli/src/merge_tools/external.rs
View file

@ -50,6 +50,8 @@ pub struct ExternalMergeTool {
/// Arguments to pass to the program when generating diffs.
/// `$left` and `$right` are replaced with the corresponding directories.
pub diff_args: Vec<String>,
/// Exit codes to be treated as success when generating diffs.
pub diff_expected_exit_codes: Vec<i32>,
/// Whether to execute the tool with a pair of directories or individual
/// files.
pub diff_invocation_mode: DiffToolMode,
@ -101,6 +103,7 @@ impl Default for ExternalMergeTool {
// `edit-args = false`, or `edit-args = []`, or `edit = { disabled =
// true }` to go with `edit = { args = [...] }`.
diff_args: ["$left", "$right"].map(ToOwned::to_owned).to_vec(),
diff_expected_exit_codes: vec![0],
edit_args: ["$left", "$right"].map(ToOwned::to_owned).to_vec(),
merge_args: vec![],
merge_conflict_exit_codes: vec![],
@ -411,7 +414,10 @@ pub fn invoke_external_diff(
// will exit with 1 if inputs are different.
let exit_status = child.wait().map_err(ExternalToolError::Io)?;
tracing::info!(?cmd, ?exit_status, "The external diff generator exited:");
if !exit_status.success() {
let exit_ok = exit_status
.code()
.is_some_and(|status| tool.diff_expected_exit_codes.contains(&status));
if !exit_ok {
writeln!(
ui.warning_default(),
"Tool exited with {exit_status} (run with --debug to see the exact invocation)",

84
cli/src/merge_tools/mod.rs
View file

@ -393,7 +393,7 @@ mod tests {
insta::assert_debug_snapshot!(get(":builtin", "").unwrap(), @"Builtin");
// Just program name, edit_args are filled by default
insta::assert_debug_snapshot!(get("my diff", "").unwrap(), @r###"
insta::assert_debug_snapshot!(get("my diff", "").unwrap(), @r#"
External(
ExternalMergeTool {
program: "my diff",
@ -401,6 +401,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"$left",
@ -412,7 +415,7 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
// Pick from merge-tools
insta::assert_debug_snapshot!(get(
@ -420,7 +423,7 @@ mod tests {
[merge-tools."foo bar"]
edit-args = ["--edit", "args", "$left", "$right"]
"#,
).unwrap(), @r###"
).unwrap(), @r#"
External(
ExternalMergeTool {
program: "foo bar",
@ -428,6 +431,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"--edit",
@ -441,7 +447,7 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
}
#[test]
@ -463,7 +469,7 @@ mod tests {
insta::assert_debug_snapshot!(get("").unwrap(), @"Builtin");
// Just program name, edit_args are filled by default
insta::assert_debug_snapshot!(get(r#"ui.diff-editor = "my-diff""#).unwrap(), @r###"
insta::assert_debug_snapshot!(get(r#"ui.diff-editor = "my-diff""#).unwrap(), @r#"
External(
ExternalMergeTool {
program: "my-diff",
@ -471,6 +477,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"$left",
@ -482,11 +491,11 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
// String args (with interpolation variables)
insta::assert_debug_snapshot!(
get(r#"ui.diff-editor = "my-diff -l $left -r $right""#).unwrap(), @r###"
get(r#"ui.diff-editor = "my-diff -l $left -r $right""#).unwrap(), @r#"
External(
ExternalMergeTool {
program: "my-diff",
@ -494,6 +503,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"-l",
@ -507,11 +519,11 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
// List args (with interpolation variables)
insta::assert_debug_snapshot!(
get(r#"ui.diff-editor = ["my-diff", "--diff", "$left", "$right"]"#).unwrap(), @r###"
get(r#"ui.diff-editor = ["my-diff", "--diff", "$left", "$right"]"#).unwrap(), @r#"
External(
ExternalMergeTool {
program: "my-diff",
@ -519,6 +531,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"--diff",
@ -531,7 +546,7 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
// Pick from merge-tools
insta::assert_debug_snapshot!(get(
@ -540,7 +555,7 @@ mod tests {
[merge-tools."foo bar"]
edit-args = ["--edit", "args", "$left", "$right"]
"#,
).unwrap(), @r###"
).unwrap(), @r#"
External(
ExternalMergeTool {
program: "foo bar",
@ -548,6 +563,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"--edit",
@ -561,7 +579,7 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
// Pick from merge-tools, but no edit-args specified
insta::assert_debug_snapshot!(get(
@ -570,7 +588,7 @@ mod tests {
[merge-tools.my-diff]
program = "MyDiff"
"#,
).unwrap(), @r###"
).unwrap(), @r#"
External(
ExternalMergeTool {
program: "MyDiff",
@ -578,6 +596,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"$left",
@ -589,10 +610,10 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
// List args should never be a merge-tools key, edit_args are filled by default
insta::assert_debug_snapshot!(get(r#"ui.diff-editor = ["meld"]"#).unwrap(), @r###"
insta::assert_debug_snapshot!(get(r#"ui.diff-editor = ["meld"]"#).unwrap(), @r#"
External(
ExternalMergeTool {
program: "meld",
@ -600,6 +621,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"$left",
@ -611,7 +635,7 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
// Invalid type
assert!(get(r#"ui.diff-editor.k = 0"#).is_err());
@ -641,7 +665,7 @@ mod tests {
[merge-tools."foo bar"]
merge-args = ["$base", "$left", "$right", "$output"]
"#,
).unwrap(), @r###"
).unwrap(), @r#"
External(
ExternalMergeTool {
program: "foo bar",
@ -649,6 +673,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"$left",
@ -665,7 +692,7 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
}
#[test]
@ -690,7 +717,7 @@ mod tests {
// String args
insta::assert_debug_snapshot!(
get(r#"ui.merge-editor = "my-merge $left $base $right $output""#).unwrap(), @r###"
get(r#"ui.merge-editor = "my-merge $left $base $right $output""#).unwrap(), @r#"
External(
ExternalMergeTool {
program: "my-merge",
@ -698,6 +725,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"$left",
@ -714,13 +744,13 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
// List args
insta::assert_debug_snapshot!(
get(
r#"ui.merge-editor = ["my-merge", "$left", "$base", "$right", "$output"]"#,
).unwrap(), @r###"
).unwrap(), @r#"
External(
ExternalMergeTool {
program: "my-merge",
@ -728,6 +758,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"$left",
@ -744,7 +777,7 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
// Pick from merge-tools
insta::assert_debug_snapshot!(get(
@ -753,7 +786,7 @@ mod tests {
[merge-tools."foo bar"]
merge-args = ["$base", "$left", "$right", "$output"]
"#,
).unwrap(), @r###"
).unwrap(), @r#"
External(
ExternalMergeTool {
program: "foo bar",
@ -761,6 +794,9 @@ mod tests {
"$left",
"$right",
],
diff_expected_exit_codes: [
0,
],
diff_invocation_mode: Dir,
edit_args: [
"$left",
@ -777,7 +813,7 @@ mod tests {
conflict_marker_style: None,
},
)
"###);
"#);
// List args should never be a merge-tools key
insta::assert_debug_snapshot!(

30
cli/tests/test_diff_command.rs
View file

@ -2042,6 +2042,36 @@ fn test_diff_external_tool() {
std::fs::write(repo_path.join("file3"), "foo\n").unwrap();
let edit_script = test_env.set_up_fake_diff_editor();
// nonzero exit codes should print a warning
std::fs::write(&edit_script, "fail").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&["diff", "--config=ui.diff.tool=fake-diff-editor"],
);
let mut insta_settings = insta::Settings::clone_current();
insta_settings.add_filter("exit (status|code)", "<exit status>");
insta_settings.bind(|| {
insta::assert_snapshot!(stdout, @r"");
insta::assert_snapshot!(stderr, @r#"
Warning: Tool exited with <exit status>: 1 (run with --debug to see the exact invocation)
"#);
});
// nonzero exit codes should not print a warning if it's an expected exit code
std::fs::write(&edit_script, "fail").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&[
"diff",
"--tool",
"fake-diff-editor",
"--config=merge-tools.fake-diff-editor.diff-expected-exit-codes=[1]",
],
);
insta::assert_snapshot!(stdout, @r"");
insta::assert_snapshot!(stderr, @r"");
std::fs::write(
&edit_script,
"print-files-before\0print --\0print-files-after",

9
docs/config.md
View file

@ -286,6 +286,15 @@ diff.tool = "vimdiff"
diff-invocation-mode = "file-by-file"
```
By default `jj` will display a warning when the command exits with a non-success
error code. The `diff-expected-exit-codes` config can suppress this warning
message for specific exit codes:
```toml
[merge-tools.delta]
diff-expected-exit-codes = [0, 1]
```
### Conflict marker style
You can configure which style of conflict markers to use when materializing