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

cli: allow to set ui.diff/merge-editor arguments inline

Browse source 
For stock merge-tools, having name -> args indirection makes sense. For
user-specific settings, it's simpler to set command name and arguments
together.

It might be a bit odd that "name with whitespace" can be parsed differently
depending on the existence of merge-tools."name with whitespace".
This commit is contained in:
Yuya Nishihara 2023-02-05 16:41:28 +09:00
parent 78b63c8137
commit 13d9dc4460
3 changed files with 128 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -114,6 +114,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
and `jj split any-non-existent-path` inserts an empty commit between the
target commit and its parents.
* Command arguments to `ui.diff-editor`/`ui.merge-editor` can now be specified
inline without referring to `[merge-tools]` table.
### Fixed bugs
* When sharing the working copy with a Git repo, we used to forget to export

14
docs/config.md
View file

@ -186,7 +186,8 @@ directories to diff are passed as the first and second argument respectively.
For example:
```toml
ui.diff-editor = "kdiff3"
ui.diff-editor = "kdiff3" # Use merge-tools.kdiff3.edit-args
ui.diff-editor = ["kdiff3", "--merge"] # Specify edit-args inline
```
Custom arguments can be added, and will be inserted before the paths to diff:
@ -212,17 +213,20 @@ The `ui.merge-editor` key specifies the tool used for three-way merge tools
by `jj resolve`. For example:
```toml
# Use merge-tools.meld.merge-args
ui.merge-editor = "meld" # Or "kdiff3" or "vimdiff"
# Specify merge-args inline
ui.merge-editor = ["meld", "$left", "$base", "$right", "-o", "$output"]
```
The "meld", "kdiff3", and "vimdiff" tools can be used out of the box, as long as
they are installed.
To use a different tool named `TOOL`, the arguments to pass to the tool MUST be
specified in the `merge-tools.TOOL.merge-args` key. As an example of how to set
this key and other tool configuration options, here is the out-of-the-box
configuration of the three default tools. (There is no need to copy it to your
config file verbatim, but you are welcome to customize it.)
specified either inline or in the `merge-tools.TOOL.merge-args` key. As an
example of how to set this key and other tool configuration options, here is
the out-of-the-box configuration of the three default tools. (There is no need
to copy it to your config file verbatim, but you are welcome to customize it.)
```toml
# merge-tools.kdiff3.program = "kdiff3" # Defaults to the name of the tool if not specified

135
src/merge_tools.rs
View file

@ -35,6 +35,7 @@ use jujutsu_lib::tree::Tree;
use jujutsu_lib::working_copy::{CheckoutError, SnapshotError, TreeState};
use thiserror::Error;
use crate::config::FullCommandArgs;
use crate::ui::Ui;
#[derive(Debug, Error)]
@ -407,19 +408,29 @@ struct MergeTool {
}
impl MergeTool {
pub fn with_program(program: &str) -> Self {
pub fn with_edit_args(args: &FullCommandArgs) -> Self {
let full_args = args.args();
MergeTool {
program: program.to_owned(),
edit_args: vec![],
program: full_args[0].to_owned(),
edit_args: full_args[1..].to_vec(),
merge_args: vec![],
merge_tool_edits_conflict_markers: false,
}
}
pub fn with_merge_args(args: &FullCommandArgs) -> Self {
let full_args = args.args();
MergeTool {
program: full_args[0].to_owned(),
edit_args: vec![],
merge_args: full_args[1..].to_vec(),
merge_tool_edits_conflict_markers: false,
}
}
}
/// Loads merge tool options from `[merge-tools.<name>]`. The given name is used
/// as an executable name if no configuration found for that name.
fn get_tool_config(settings: &UserSettings, name: &str) -> Result<MergeTool, ConfigError> {
/// Loads merge tool options from `[merge-tools.<name>]`.
fn get_tool_config(settings: &UserSettings, name: &str) -> Result<Option<MergeTool>, ConfigError> {
const TABLE_KEY: &str = "merge-tools";
let tools_table = settings.config().get_table(TABLE_KEY)?;
if let Some(v) = tools_table.get(name) {
@ -432,9 +443,9 @@ fn get_tool_config(settings: &UserSettings, name: &str) -> Result<MergeTool, Con
if result.program.is_empty() {
result.program.clone_from(&name.to_string());
};
Ok(result)
Ok(Some(result))
} else {
Ok(MergeTool::with_program(name))
Ok(None)
}
}
@ -442,19 +453,27 @@ fn get_diff_editor_from_settings(
ui: &mut Ui,
settings: &UserSettings,
) -> Result<MergeTool, ExternalToolError> {
let editor_name = editor_name_from_settings(ui, settings, "ui.diff-editor")?;
Ok(get_tool_config(settings, &editor_name)?)
let args = editor_args_from_settings(ui, settings, "ui.diff-editor")?;
let maybe_editor = match &args {
FullCommandArgs::String(name) => get_tool_config(settings, name)?,
FullCommandArgs::Vec(_) => None,
};
Ok(maybe_editor.unwrap_or_else(|| MergeTool::with_edit_args(&args)))
}
fn get_merge_tool_from_settings(
ui: &mut Ui,
settings: &UserSettings,
) -> Result<MergeTool, ExternalToolError> {
let editor_name = editor_name_from_settings(ui, settings, "ui.merge-editor")?;
let editor = get_tool_config(settings, &editor_name)?;
let args = editor_args_from_settings(ui, settings, "ui.merge-editor")?;
let maybe_editor = match &args {
FullCommandArgs::String(name) => get_tool_config(settings, name)?,
FullCommandArgs::Vec(_) => None,
};
let editor = maybe_editor.unwrap_or_else(|| MergeTool::with_merge_args(&args));
if editor.merge_args.is_empty() {
Err(ExternalToolError::MergeArgsNotConfigured {
tool_name: editor_name,
tool_name: args.to_string(),
})
} else {
Ok(editor)
@ -462,22 +481,22 @@ fn get_merge_tool_from_settings(
}
/// Finds the appropriate tool for diff editing or merges
fn editor_name_from_settings(
fn editor_args_from_settings(
ui: &mut Ui,
settings: &UserSettings,
key: &str,
) -> Result<String, ExternalToolError> {
) -> Result<FullCommandArgs, ExternalToolError> {
// TODO: Make this configuration have a table of possible editors and detect the
// best one here.
match settings.config().get_string(key) {
Ok(editor_binary) => Ok(editor_binary),
match settings.config().get::<FullCommandArgs>(key) {
Ok(args) => Ok(args),
Err(config::ConfigError::NotFound(_)) => {
let default_editor = "meld".to_string();
let default_editor = "meld";
writeln!(
ui.hint(),
"Using default editor '{default_editor}'; you can change this by setting {key}"
)?;
Ok(default_editor)
Ok(default_editor.into())
}
Err(err) => Err(err.into()),
}
@ -532,6 +551,32 @@ mod tests {
}
"###);
// String args
insta::assert_debug_snapshot!(
get(r#"ui.diff-editor = "my-diff --diff""#).unwrap(), @r###"
MergeTool {
program: "my-diff",
edit_args: [
"--diff",
],
merge_args: [],
merge_tool_edits_conflict_markers: false,
}
"###);
// List args
insta::assert_debug_snapshot!(
get(r#"ui.diff-editor = ["my-diff", "--diff"]"#).unwrap(), @r###"
MergeTool {
program: "my-diff",
edit_args: [
"--diff",
],
merge_args: [],
merge_tool_edits_conflict_markers: false,
}
"###);
// Pick from merge-tools
insta::assert_debug_snapshot!(get(
r#"
@ -551,6 +596,16 @@ mod tests {
}
"###);
// List args should never be a merge-tools key
insta::assert_debug_snapshot!(get(r#"ui.diff-editor = ["meld"]"#).unwrap(), @r###"
MergeTool {
program: "meld",
edit_args: [],
merge_args: [],
merge_tool_edits_conflict_markers: false,
}
"###);
// Invalid type
assert!(get(r#"ui.diff-editor.k = 0"#).is_err());
}
@ -588,6 +643,40 @@ mod tests {
}
"###);
// String args
insta::assert_debug_snapshot!(
get(r#"ui.merge-editor = "my-merge $left $base $right $output""#).unwrap(), @r###"
MergeTool {
program: "my-merge",
edit_args: [],
merge_args: [
"$left",
"$base",
"$right",
"$output",
],
merge_tool_edits_conflict_markers: false,
}
"###);
// List args
insta::assert_debug_snapshot!(
get(
r#"ui.merge-editor = ["my-merge", "$left", "$base", "$right", "$output"]"#,
).unwrap(), @r###"
MergeTool {
program: "my-merge",
edit_args: [],
merge_args: [
"$left",
"$base",
"$right",
"$output",
],
merge_tool_edits_conflict_markers: false,
}
"###);
// Pick from merge-tools
insta::assert_debug_snapshot!(get(
r#"
@ -609,6 +698,14 @@ mod tests {
}
"###);
// List args should never be a merge-tools key
insta::assert_debug_snapshot!(
get(r#"ui.merge-editor = ["meld"]"#).unwrap_err(), @r###"
MergeArgsNotConfigured {
tool_name: "meld",
}
"###);
// Invalid type
assert!(get(r#"ui.merge-editor.k = 0"#).is_err());
}