Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: add option to generate textual diff by external command #1961

Merged
merged 7 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
from a merge revision's parents. This undoes the changes that `jj diff -r`
would show.

* `jj diff`/`log` now supports `--tool <name>` option to generate diffs by
external program. For configuration, see [the documentation](docs/config.md).
[#1886](https://github.com/martinvonz/jj/issues/1886)

### Fixed bugs

Expand Down
15 changes: 15 additions & 0 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,21 @@ ui.default-command = "log"
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.

```toml
[merge-tools.<name>]
# program = "<name>" # Defaults to the name of the tool if not specified
diff-args = ["--color=always", "$left", "$right"]
```

- `$left` and `$right` are replaced with the paths to the left and right
directories to diff respectively.

### Default revisions to log

You can configure the revisions `jj log` without `-r` should show.
Expand Down
4 changes: 4 additions & 0 deletions lib/src/working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@ pub enum TreeStateError {
}

impl TreeState {
pub fn working_copy_path(&self) -> &Path {
&self.working_copy_path
}

pub fn current_tree_id(&self) -> &TreeId {
&self.tree_id
}
Expand Down
8 changes: 7 additions & 1 deletion src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ use crate::config::{
new_config_path, AnnotatedValue, CommandNameAndArgs, ConfigSource, LayeredConfigs,
};
use crate::formatter::{FormatRecorder, Formatter, PlainTextFormatter};
use crate::merge_tools::{ConflictResolveError, DiffEditError};
use crate::merge_tools::{ConflictResolveError, DiffEditError, DiffGenerateError};
use crate::template_parser::{TemplateAliasesMap, TemplateParseError};
use crate::templater::Template;
use crate::ui::{ColorChoice, Ui};
Expand Down Expand Up @@ -239,6 +239,12 @@ impl From<DiffEditError> for CommandError {
}
}

impl From<DiffGenerateError> for CommandError {
fn from(err: DiffGenerateError) -> Self {
user_error(format!("Failed to generate diff: {err}"))
}
}

impl From<ConflictResolveError> for CommandError {
fn from(err: ConflictResolveError) -> Self {
user_error(format!("Failed to use external tool to resolve: {err}"))
Expand Down
13 changes: 8 additions & 5 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1445,14 +1445,15 @@ fn cmd_diff(ui: &mut Ui, command: &CommandHelper, args: &DiffArgs) -> Result<(),
to_tree = commit.tree()
}
let matcher = workspace_command.matcher_from_values(&args.paths)?;
let diff_formats = diff_util::diff_formats_for(command.settings(), &args.format)?;
ui.request_pager();
diff_util::show_diff(
ui.stdout_formatter().as_mut(),
&workspace_command,
&from_tree,
&to_tree,
matcher.as_ref(),
&diff_util::diff_formats_for(command.settings(), &args.format),
&diff_formats,
)?;
Ok(())
}
Expand All @@ -1463,6 +1464,7 @@ fn cmd_show(ui: &mut Ui, command: &CommandHelper, args: &ShowArgs) -> Result<(),
let commit = workspace_command.resolve_single_rev(&args.revision, ui)?;
let template_string = command.settings().config().get_string("templates.show")?;
let template = workspace_command.parse_commit_template(&template_string)?;
let diff_formats = diff_util::diff_formats_for(command.settings(), &args.format)?;
ui.request_pager();
let mut formatter = ui.stdout_formatter();
let formatter = formatter.as_mut();
Expand All @@ -1472,7 +1474,7 @@ fn cmd_show(ui: &mut Ui, command: &CommandHelper, args: &ShowArgs) -> Result<(),
&workspace_command,
&commit,
&EverythingMatcher,
&diff_util::diff_formats_for(command.settings(), &args.format),
&diff_formats,
)?;
Ok(())
}
Expand Down Expand Up @@ -1609,7 +1611,7 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C

let store = repo.store();
let diff_formats =
diff_util::diff_formats_for_log(command.settings(), &args.diff_format, args.patch);
diff_util::diff_formats_for_log(command.settings(), &args.diff_format, args.patch)?;

let template_string = match &args.template {
Some(value) => value.to_string(),
Expand Down Expand Up @@ -1745,7 +1747,7 @@ fn cmd_obslog(ui: &mut Ui, command: &CommandHelper, args: &ObslogArgs) -> Result
let wc_commit_id = workspace_command.get_wc_commit_id();

let diff_formats =
diff_util::diff_formats_for_log(command.settings(), &args.diff_format, args.patch);
diff_util::diff_formats_for_log(command.settings(), &args.diff_format, args.patch)?;

let template_string = match &args.template {
Some(value) => value.to_string(),
Expand Down Expand Up @@ -1849,14 +1851,15 @@ fn cmd_interdiff(

let from_tree = rebase_to_dest_parent(&workspace_command, &from, &to)?;
let matcher = workspace_command.matcher_from_values(&args.paths)?;
let diff_formats = diff_util::diff_formats_for(command.settings(), &args.format)?;
ui.request_pager();
diff_util::show_diff(
ui.stdout_formatter().as_mut(),
&workspace_command,
&from_tree,
&to.tree(),
matcher.as_ref(),
&diff_util::diff_formats_for(command.settings(), &args.format),
&diff_formats,
)
}

Expand Down
6 changes: 6 additions & 0 deletions src/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,12 @@
"program": {
"type": "string"
},
"diff-args": {
"type": "array",
"items": {
"type": "string"
}
},
"edit-args": {
"type": "array",
"items": {
Expand Down
84 changes: 56 additions & 28 deletions src/diff_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,18 @@ use jj_lib::files::DiffLine;
use jj_lib::matchers::Matcher;
use jj_lib::repo::{ReadonlyRepo, Repo};
use jj_lib::repo_path::RepoPath;
use jj_lib::settings::UserSettings;
use jj_lib::settings::{ConfigResultExt as _, UserSettings};
use jj_lib::tree::{Tree, TreeDiffIterator};
use jj_lib::{diff, files, rewrite, tree};
use tracing::instrument;

use crate::cli_util::{CommandError, WorkspaceCommandHelper};
use crate::formatter::Formatter;
use crate::merge_tools::{self, MergeTool};

#[derive(clap::Args, Clone, Debug)]
#[command(group(clap::ArgGroup::new("short-format").args(&["summary", "types"])))]
#[command(group(clap::ArgGroup::new("long-format").args(&["git", "color_words"])))]
#[command(group(clap::ArgGroup::new("long-format").args(&["git", "color_words", "tool"])))]
pub struct DiffFormatArgs {
/// For each path, show only whether it was modified, added, or removed
#[arg(long, short)]
Expand All @@ -55,23 +56,30 @@ pub struct DiffFormatArgs {
/// Show a word-level diff with changes indicated only by color
#[arg(long)]
pub color_words: bool,
/// Generate diff by external command
#[arg(long)]
pub tool: Option<String>,
}

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum DiffFormat {
Summary,
Types,
Git,
ColorWords,
Tool(Box<MergeTool>),
}

/// Returns a list of requested diff formats, which will never be empty.
pub fn diff_formats_for(settings: &UserSettings, args: &DiffFormatArgs) -> Vec<DiffFormat> {
let formats = diff_formats_from_args(args);
pub fn diff_formats_for(
settings: &UserSettings,
args: &DiffFormatArgs,
) -> Result<Vec<DiffFormat>, config::ConfigError> {
let formats = diff_formats_from_args(settings, args)?;
if formats.is_empty() {
vec![default_diff_format(settings)]
Ok(vec![default_diff_format(settings)?])
} else {
formats
Ok(formats)
}
}

Expand All @@ -81,41 +89,55 @@ pub fn diff_formats_for_log(
settings: &UserSettings,
args: &DiffFormatArgs,
patch: bool,
) -> Vec<DiffFormat> {
let mut formats = diff_formats_from_args(args);
) -> Result<Vec<DiffFormat>, config::ConfigError> {
let mut formats = diff_formats_from_args(settings, args)?;
// --patch implies default if no format other than --summary is specified
if patch && matches!(formats.as_slice(), [] | [DiffFormat::Summary]) {
formats.push(default_diff_format(settings));
formats.push(default_diff_format(settings)?);
formats.dedup();
}
formats
Ok(formats)
}

fn diff_formats_from_args(args: &DiffFormatArgs) -> Vec<DiffFormat> {
[
fn diff_formats_from_args(
settings: &UserSettings,
args: &DiffFormatArgs,
) -> Result<Vec<DiffFormat>, config::ConfigError> {
let mut formats = [
(args.summary, DiffFormat::Summary),
(args.types, DiffFormat::Types),
(args.git, DiffFormat::Git),
(args.color_words, DiffFormat::ColorWords),
]
.into_iter()
.filter_map(|(arg, format)| arg.then_some(format))
.collect()
.collect_vec();
if let Some(name) = &args.tool {
let tool = merge_tools::get_tool_config(settings, name)?
.unwrap_or_else(|| MergeTool::with_program(name));
formats.push(DiffFormat::Tool(Box::new(tool)));
}
Ok(formats)
}

fn default_diff_format(settings: &UserSettings) -> DiffFormat {
match settings
.config()
.get_string("ui.diff.format")
// old config name
.or_else(|_| settings.config().get_string("diff.format"))
.as_deref()
{
Ok("summary") => DiffFormat::Summary,
Ok("types") => DiffFormat::Types,
Ok("git") => DiffFormat::Git,
Ok("color-words") => DiffFormat::ColorWords,
_ => DiffFormat::ColorWords,
fn default_diff_format(settings: &UserSettings) -> Result<DiffFormat, config::ConfigError> {
let config = settings.config();
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()? {
name // old config name
} else {
"color-words".to_owned()
};
match name.as_ref() {
"summary" => Ok(DiffFormat::Summary),
"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}"
))),
}
}

Expand All @@ -128,20 +150,26 @@ pub fn show_diff(
formats: &[DiffFormat],
) -> Result<(), CommandError> {
for format in formats {
let tree_diff = from_tree.diff(to_tree, matcher);
match format {
DiffFormat::Summary => {
let tree_diff = from_tree.diff(to_tree, matcher);
show_diff_summary(formatter, workspace_command, tree_diff)?;
}
DiffFormat::Types => {
let tree_diff = from_tree.diff(to_tree, matcher);
show_types(formatter, workspace_command, tree_diff)?;
}
DiffFormat::Git => {
let tree_diff = from_tree.diff(to_tree, matcher);
show_git_diff(formatter, workspace_command, tree_diff)?;
}
DiffFormat::ColorWords => {
let tree_diff = from_tree.diff(to_tree, matcher);
show_color_words_diff(formatter, workspace_command, tree_diff)?;
}
DiffFormat::Tool(tool) => {
merge_tools::generate_diff(formatter.raw(), from_tree, to_tree, matcher, tool)?;
}
}
}
Ok(())
Expand Down
Loading