Skip to content

Commit

Permalink
diff_edit: load command arguments from merge-tools.<name> config
Browse files Browse the repository at this point in the history
Apparently, I need to pass `--merge` option to use kdiff3 as a diff editor.

We could add `diff-editor-args` or extend `diff-editor` to a list of command
arguments, but we'll eventually add stock merge tools and the configuration
would look like:

    [merge-tools.<name>]
    program = ...
    diff-args = [...]
    edit-args = [...]
    merge-args = [...]
  • Loading branch information
yuja committed May 3, 2022
1 parent 1f2753f commit f6acee4
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 4 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
will create a branch named after the revision's change ID, so you don't have
to create a branch yourself.

* Diff editor command arguments can now be specified by config file.
Example:

[merge-tools.kdiff3]
program = "kdiff3"
edit-args = ["--merge", "--cs", "CreateBakFiles=0"]

### Fixed bugs

* When rebasing a conflict where one side modified a file and the other side
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pest = "2.1.3"
pest_derive = "2.1.0"
rand = "0.8.5"
regex = "1.5.5"
serde = { version = "1.0", features = ["derive"] }
tempfile = "3.3.0"
textwrap = "0.15.0"
thiserror = "1.0.31"
Expand Down
51 changes: 47 additions & 4 deletions src/diff_edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use std::path::{Path, PathBuf};
use std::process::Command;
use std::sync::Arc;

use config::ConfigError;
use itertools::Itertools;
use jujutsu_lib::backend::TreeId;
use jujutsu_lib::gitignore::GitIgnoreFile;
Expand All @@ -34,6 +35,8 @@ use crate::ui::Ui;

#[derive(Debug, Error)]
pub enum DiffEditError {
#[error("Invalid config: {0}")]
ConfigError(#[from] ConfigError),
#[error("The diff tool exited with a non-zero code")]
DifftoolAborted,
#[error("Failed to write directories to diff: {0:?}")]
Expand Down Expand Up @@ -138,7 +141,7 @@ pub fn edit_diff(

// TODO: Make this configuration have a table of possible editors and detect the
// best one here.
let editor_binary = match settings.config().get_string("ui.diff-editor") {
let editor_name = match settings.config().get_string("ui.diff-editor") {
Ok(editor_binary) => editor_binary,
Err(_) => {
let default_editor = "meld".to_string();
Expand All @@ -150,14 +153,15 @@ pub fn edit_diff(
default_editor
}
};

let editor = get_tool(settings, &editor_name)?;
// Start a diff editor on the two directories.
let exit_status = Command::new(&editor_binary)
let exit_status = Command::new(&editor.program)
.args(&editor.edit_args)
.arg(&left_wc_dir)
.arg(&right_wc_dir)
.status()
.map_err(|e| DiffEditError::ExecuteEditorError {
editor_binary,
editor_binary: editor.program,
source: e,
})?;
if !exit_status.success() {
Expand All @@ -169,3 +173,42 @@ pub fn edit_diff(

Ok(right_tree_state.snapshot(base_ignores)?)
}

/// Merge/diff tool loaded from the settings.
#[derive(Clone, Debug, serde::Deserialize)]
#[serde(rename_all = "kebab-case")]
struct MergeTool {
/// Program to execute.
pub program: String,
/// Arguments to pass to the program when editing diffs.
#[serde(default)]
pub edit_args: Vec<String>,
}

impl MergeTool {
pub fn with_program(program: &str) -> Self {
MergeTool {
program: program.to_owned(),
edit_args: vec![],
}
}
}

/// 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(settings: &UserSettings, name: &str) -> Result<MergeTool, ConfigError> {
const TABLE_KEY: &'static str = "merge-tools";
let tools_table = match settings.config().get_table(TABLE_KEY) {
Ok(table) => table,
Err(ConfigError::NotFound(_)) => return Ok(MergeTool::with_program(name)),
Err(err) => return Err(err),
};
if let Some(v) = tools_table.get(name) {
v.clone()
.try_deserialize()
// add config key, deserialize error is otherwise unclear
.map_err(|e| ConfigError::Message(format!("{TABLE_KEY}.{name}: {e}")))
} else {
Ok(MergeTool::with_program(name))
}
}

0 comments on commit f6acee4

Please sign in to comment.