ok/jj
1
0
Fork 0
forked from mirrors/jj

cli: load default diff command from "ui.diff.tool" config

This could be a "ui.diff.format" variant, but we would need to solve namespace
problem by prefixing "tool:<name>" for example. Then, inlining command arguments
would become uglier.

#1886
This commit is contained in:
Yuya Nishihara 2023-08-04 07:25:22 +09:00
parent 962cd25821
commit b352061dd6
6 changed files with 57 additions and 10 deletions

View file

@ -143,9 +143,18 @@ ui.diff.format = "git"
### Generating diffs by external command
If `diff --tool <name>` argument is given, the external diff command will be
called instead of the internal diff function. The command arguments can be
specified as follows.
If `ui.diff.tool` is set, the specified diff command will be called instead of
the internal diff function.
```toml
# Use Difftastic by default
ui.diff.tool = ["difft", "--color=always", "$left", "$right"]
# Use tool named "<name>" (see below)
ui.diff.tool = "<name>"
```
The external diff tool can also be enabled by `diff --tool <name>` argument.
For the tool named `<name>`, command arguments can be configured as follows.
```toml
[merge-tools.<name>]

View file

@ -82,6 +82,10 @@
"summary"
],
"default": "color-words"
},
"tool": {
"type": "string",
"description": "External tool for generating diffs"
}
}
},

View file

@ -122,6 +122,12 @@ fn diff_formats_from_args(
fn default_diff_format(settings: &UserSettings) -> Result<DiffFormat, config::ConfigError> {
let config = settings.config();
if let Some(args) = config.get("ui.diff.tool").optional()? {
// External "tool" overrides the internal "format" option.
let tool = merge_tools::get_tool_config_from_args(settings, &args)?
.unwrap_or_else(|| MergeTool::with_diff_args(&args));
return Ok(DiffFormat::Tool(Box::new(tool)));
}
let name = if let Some(name) = config.get_string("ui.diff.format").optional()? {
name
} else if let Some(name) = config.get_string("diff.format").optional()? {
@ -134,7 +140,6 @@ fn default_diff_format(settings: &UserSettings) -> Result<DiffFormat, config::Co
"types" => Ok(DiffFormat::Types),
"git" => Ok(DiffFormat::Git),
"color-words" => Ok(DiffFormat::ColorWords),
// TODO: add configurable default for DiffFormat::Tool?
_ => Err(config::ConfigError::Message(format!(
"invalid diff format: {name}"
))),

View file

@ -509,6 +509,10 @@ impl MergeTool {
}
}
pub fn with_diff_args(command_args: &CommandNameAndArgs) -> Self {
Self::with_args_inner(command_args, |tool| &mut tool.diff_args)
}
pub fn with_edit_args(command_args: &CommandNameAndArgs) -> Self {
Self::with_args_inner(command_args, |tool| &mut tool.edit_args)
}
@ -558,7 +562,7 @@ pub fn get_tool_config(
/// Loads merge tool options from `[merge-tools.<name>]` if `args` is of
/// unstructured string type.
fn get_tool_config_from_args(
pub fn get_tool_config_from_args(
settings: &UserSettings,
args: &CommandNameAndArgs,
) -> Result<Option<MergeTool>, ConfigError> {

View file

@ -250,11 +250,7 @@ impl TestEnvironment {
/// Sets up the fake diff-editor to read an edit script from the returned
/// path
pub fn set_up_fake_diff_editor(&mut self) -> PathBuf {
let diff_editor_path = assert_cmd::cargo::cargo_bin("fake-diff-editor");
assert!(diff_editor_path.is_file());
// Simplified TOML escaping, hoping that there are no '"' or control characters
// in it
let escaped_diff_editor_path = diff_editor_path.to_str().unwrap().replace('\\', r"\\");
let escaped_diff_editor_path = escaped_fake_diff_editor_path();
self.add_config(&format!(
r###"
ui.diff-editor = "fake-diff-editor"
@ -301,3 +297,11 @@ pub fn get_stdout_string(assert: &assert_cmd::assert::Assert) -> String {
pub fn get_stderr_string(assert: &assert_cmd::assert::Assert) -> String {
String::from_utf8(assert.get_output().stderr.clone()).unwrap()
}
pub fn escaped_fake_diff_editor_path() -> String {
let diff_editor_path = assert_cmd::cargo::cargo_bin("fake-diff-editor");
assert!(diff_editor_path.is_file());
// Simplified TOML escaping, hoping that there are no '"' or control characters
// in it
diff_editor_path.to_str().unwrap().replace('\\', r"\\")
}

View file

@ -647,6 +647,27 @@ fn test_diff_external_tool() {
file3
"###);
// Enabled by default, looks up the merge-tools table
let config = "--config-toml=ui.diff.tool='fake-diff-editor'";
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", config]), @r###"
file1
file2
--
file2
file3
"###);
// Inlined command arguments
let command = common::escaped_fake_diff_editor_path();
let config = format!(r#"--config-toml=ui.diff.tool=["{command}", "$right", "$left"]"#);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", &config]), @r###"
file2
file3
--
file1
file2
"###);
// Output of external diff tool shouldn't be escaped
std::fs::write(&edit_script, "print \x1b[1;31mred").unwrap();
insta::assert_snapshot!(