Skip to content

Commit

Permalink
Add --context flag for diffs.
Browse files Browse the repository at this point in the history
Allows specifying the number of lines of context to show around diffs.
The logic was already in place, just some plumbing was needed.
  • Loading branch information
algmyr committed Mar 2, 2024
1 parent b926fd8 commit bedea06
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 20 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `jj branch list` now supports a `--tracked/-t` option which can be used to
show tracked branches only. Omits local Git-tracking branches by default.

* Commands producing diffs now accept a `--context` flag for the number of
lines of context to show.

### Fixed bugs

* On Windows, symlinks in the repo are now materialized as regular files in the
Expand Down
81 changes: 61 additions & 20 deletions cli/src/diff_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ use crate::merge_tools::{self, ExternalMergeTool};
use crate::text_util;
use crate::ui::Ui;

const DEFAULT_CONTEXT_LINES: usize = 3;

#[derive(clap::Args, Clone, Debug)]
#[command(next_help_heading = "Diff Formatting Options")]
#[command(group(clap::ArgGroup::new("short-format").args(&["summary", "stat", "types"])))]
Expand Down Expand Up @@ -73,15 +75,18 @@ pub struct DiffFormatArgs {
/// Generate diff by external command
#[arg(long)]
pub tool: Option<String>,
/// Number of lines of context to show
#[arg(long)]
context: Option<usize>,
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub enum DiffFormat {
Summary,
Stat,
Types,
Git,
ColorWords,
Git { context: usize },
ColorWords { context: usize },
Tool(Box<ExternalMergeTool>),
}

Expand All @@ -92,7 +97,7 @@ pub fn diff_formats_for(
) -> Result<Vec<DiffFormat>, config::ConfigError> {
let formats = diff_formats_from_args(settings, args)?;
if formats.is_empty() {
Ok(vec![default_diff_format(settings)?])
Ok(vec![default_diff_format(settings, args.context)?])
} else {
Ok(formats)
}
Expand All @@ -108,7 +113,7 @@ pub fn diff_formats_for_log(
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, args.context)?);
formats.dedup();
}
Ok(formats)
Expand All @@ -121,8 +126,18 @@ fn diff_formats_from_args(
let mut formats = [
(args.summary, DiffFormat::Summary),
(args.types, DiffFormat::Types),
(args.git, DiffFormat::Git),
(args.color_words, DiffFormat::ColorWords),
(
args.git,
DiffFormat::Git {
context: args.context.unwrap_or(DEFAULT_CONTEXT_LINES),
},
),
(
args.color_words,
DiffFormat::ColorWords {
context: args.context.unwrap_or(DEFAULT_CONTEXT_LINES),
},
),
(args.stat, DiffFormat::Stat),
]
.into_iter()
Expand All @@ -136,7 +151,10 @@ fn diff_formats_from_args(
Ok(formats)
}

fn default_diff_format(settings: &UserSettings) -> Result<DiffFormat, config::ConfigError> {
fn default_diff_format(
settings: &UserSettings,
num_context_lines: Option<usize>,
) -> 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.
Expand All @@ -158,8 +176,12 @@ fn default_diff_format(settings: &UserSettings) -> Result<DiffFormat, config::Co
match name.as_ref() {
"summary" => Ok(DiffFormat::Summary),
"types" => Ok(DiffFormat::Types),
"git" => Ok(DiffFormat::Git),
"color-words" => Ok(DiffFormat::ColorWords),
"git" => Ok(DiffFormat::Git {
context: num_context_lines.unwrap_or(DEFAULT_CONTEXT_LINES),
}),
"color-words" => Ok(DiffFormat::ColorWords {
context: num_context_lines.unwrap_or(DEFAULT_CONTEXT_LINES),
}),
"stat" => Ok(DiffFormat::Stat),
_ => Err(config::ConfigError::Message(format!(
"invalid diff format: {name}"
Expand Down Expand Up @@ -190,13 +212,13 @@ pub fn show_diff(
let tree_diff = from_tree.diff_stream(to_tree, matcher);
show_types(formatter, workspace_command, tree_diff)?;
}
DiffFormat::Git => {
DiffFormat::Git { context } => {
let tree_diff = from_tree.diff_stream(to_tree, matcher);
show_git_diff(formatter, workspace_command, tree_diff)?;
show_git_diff(formatter, workspace_command, *context, tree_diff)?;
}
DiffFormat::ColorWords => {
DiffFormat::ColorWords { context } => {
let tree_diff = from_tree.diff_stream(to_tree, matcher);
show_color_words_diff(formatter, workspace_command, tree_diff)?;
show_color_words_diff(formatter, workspace_command, *context, tree_diff)?;
}
DiffFormat::Tool(tool) => {
merge_tools::generate_diff(ui, formatter.raw(), from_tree, to_tree, matcher, tool)?;
Expand Down Expand Up @@ -231,10 +253,10 @@ pub fn show_patch(
fn show_color_words_diff_hunks(
left: &[u8],
right: &[u8],
num_context_lines: usize,
formatter: &mut dyn Formatter,
) -> io::Result<()> {
const SKIPPED_CONTEXT_LINE: &str = " ...\n";
let num_context_lines = 3;
let mut context = VecDeque::new();
// Have we printed "..." for any skipped context?
let mut skipped_context = false;
Expand Down Expand Up @@ -429,6 +451,7 @@ fn basic_diff_file_type(value: &MaterializedTreeValue) -> &'static str {
pub fn show_color_words_diff(
formatter: &mut dyn Formatter,
workspace_command: &WorkspaceCommandHelper,
num_context_lines: usize,
tree_diff: TreeDiffStream,
) -> Result<(), CommandError> {
formatter.push_label("diff")?;
Expand All @@ -449,7 +472,12 @@ pub fn show_color_words_diff(
} else if right_content.is_binary {
writeln!(formatter.labeled("binary"), " (binary)")?;
} else {
show_color_words_diff_hunks(&[], &right_content.contents, formatter)?;
show_color_words_diff_hunks(
&[],
&right_content.contents,
num_context_lines,
formatter,
)?;
}
} else if right_value.is_present() {
let description = match (&left_value, &right_value) {
Expand Down Expand Up @@ -508,6 +536,7 @@ pub fn show_color_words_diff(
show_color_words_diff_hunks(
&left_content.contents,
&right_content.contents,
num_context_lines,
formatter,
)?;
}
Expand All @@ -523,7 +552,12 @@ pub fn show_color_words_diff(
} else if left_content.is_binary {
writeln!(formatter.labeled("binary"), " (binary)")?;
} else {
show_color_words_diff_hunks(&left_content.contents, &[], formatter)?;
show_color_words_diff_hunks(
&left_content.contents,
&[],
num_context_lines,
formatter,
)?;
}
}
}
Expand Down Expand Up @@ -694,8 +728,9 @@ fn show_unified_diff_hunks(
formatter: &mut dyn Formatter,
left_content: &[u8],
right_content: &[u8],
num_context_lines: usize,
) -> Result<(), CommandError> {
for hunk in unified_diff_hunks(left_content, right_content, 3) {
for hunk in unified_diff_hunks(left_content, right_content, num_context_lines) {
writeln!(
formatter.labeled("hunk_header"),
"@@ -{},{} +{},{} @@",
Expand Down Expand Up @@ -760,6 +795,7 @@ fn materialized_diff_stream<'a>(
pub fn show_git_diff(
formatter: &mut dyn Formatter,
workspace_command: &WorkspaceCommandHelper,
num_context_lines: usize,
tree_diff: TreeDiffStream,
) -> Result<(), CommandError> {
formatter.push_label("diff")?;
Expand All @@ -778,7 +814,7 @@ pub fn show_git_diff(
writeln!(formatter, "--- /dev/null")?;
writeln!(formatter, "+++ b/{path_string}")
})?;
show_unified_diff_hunks(formatter, &[], &right_part.content)?;
show_unified_diff_hunks(formatter, &[], &right_part.content, num_context_lines)?;
} else if right_value.is_present() {
let left_part = git_diff_part(&path, left_value)?;
let right_part = git_diff_part(&path, right_value)?;
Expand All @@ -803,7 +839,12 @@ pub fn show_git_diff(
}
Ok(())
})?;
show_unified_diff_hunks(formatter, &left_part.content, &right_part.content)?;
show_unified_diff_hunks(
formatter,
&left_part.content,
&right_part.content,
num_context_lines,
)?;
} else {
let left_part = git_diff_part(&path, left_value)?;
formatter.with_label("file_header", |formatter| {
Expand All @@ -813,7 +854,7 @@ pub fn show_git_diff(
writeln!(formatter, "--- a/{path_string}")?;
writeln!(formatter, "+++ /dev/null")
})?;
show_unified_diff_hunks(formatter, &left_part.content, &[])?;
show_unified_diff_hunks(formatter, &left_part.content, &[], num_context_lines)?;
}
}
Ok::<(), CommandError>(())
Expand Down
5 changes: 5 additions & 0 deletions cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,7 @@ With the `--from` and/or `--to` options, shows the difference from/to the given
Possible values: `true`, `false`
* `--tool <TOOL>` — Generate diff by external command
* `--context <CONTEXT>` — Number of lines of context to show
Expand Down Expand Up @@ -1000,6 +1001,7 @@ This excludes changes from other commits by temporarily rebasing `--from` onto `
Possible values: `true`, `false`
* `--tool <TOOL>` — Generate diff by external command
* `--context <CONTEXT>` — Number of lines of context to show
Expand Down Expand Up @@ -1051,6 +1053,7 @@ Show commit history
Possible values: `true`, `false`
* `--tool <TOOL>` — Generate diff by external command
* `--context <CONTEXT>` — Number of lines of context to show
Expand Down Expand Up @@ -1221,6 +1224,7 @@ Show how a change has evolved as it's been updated, rebased, etc.
Possible values: `true`, `false`
* `--tool <TOOL>` — Generate diff by external command
* `--context <CONTEXT>` — Number of lines of context to show
Expand Down Expand Up @@ -1592,6 +1596,7 @@ Show commit description and changes in a revision
Possible values: `true`, `false`
* `--tool <TOOL>` — Generate diff by external command
* `--context <CONTEXT>` — Number of lines of context to show
Expand Down
103 changes: 103 additions & 0 deletions cli/tests/test_diff_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ fn test_diff_basic() {
1: foo
"###);

let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--context=0"]);
insta::assert_snapshot!(stdout, @r###"
Removed regular file file1:
1 : foo
Modified regular file file2:
1 1: foo
2: bar
Added regular file file3:
1: foo
"###);

let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s"]);
insta::assert_snapshot!(stdout, @r###"
D file1
Expand Down Expand Up @@ -79,6 +90,30 @@ fn test_diff_basic() {
+foo
"###);

let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "--context=0"]);
insta::assert_snapshot!(stdout, @r###"
diff --git a/file1 b/file1
deleted file mode 100644
index 257cc5642c..0000000000
--- a/file1
+++ /dev/null
@@ -1,1 +1,0 @@
-foo
diff --git a/file2 b/file2
index 257cc5642c...3bd1f0e297 100644
--- a/file2
+++ b/file2
@@ -2,0 +2,1 @@
+bar
diff --git a/file3 b/file3
new file mode 100644
index 0000000000..257cc5642c
--- /dev/null
+++ b/file3
@@ -1,0 +1,1 @@
+foo
"###);

let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s", "--git"]);
insta::assert_snapshot!(stdout, @r###"
D file1
Expand Down Expand Up @@ -637,6 +672,74 @@ fn test_diff_skipped_context() {
"###);
}

#[test]
fn test_diff_skipped_context_nondefault() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");

std::fs::write(repo_path.join("file1"), "a\nb\nc\nd").unwrap();
test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "=== Left side of diffs"]);

test_env.jj_cmd_ok(&repo_path, &["new", "@", "-m", "=== Must skip 2 lines"]);
std::fs::write(repo_path.join("file1"), "A\nb\nc\nD").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new", "@-", "-m", "=== Don't skip 1 line"]);
std::fs::write(repo_path.join("file1"), "A\nb\nC\nd").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new", "@-", "-m", "=== No gap to skip"]);
std::fs::write(repo_path.join("file1"), "a\nB\nC\nd").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new", "@-", "-m", "=== 1 line at start"]);
std::fs::write(repo_path.join("file1"), "a\nB\nc\nd").unwrap();
test_env.jj_cmd_ok(&repo_path, &["new", "@-", "-m", "=== 1 line at end"]);
std::fs::write(repo_path.join("file1"), "a\nb\nC\nd").unwrap();

let stdout = test_env.jj_cmd_success(
&repo_path,
&[
"log",
"-Tdescription",
"-p",
"--no-graph",
"--reversed",
"--context=0",
],
);
insta::assert_snapshot!(stdout, @r###"
=== Left side of diffs
Added regular file file1:
1: a
2: b
3: c
4: d
=== Must skip 2 lines
Modified regular file file1:
1 1: aA
...
4 4: dD
=== Don't skip 1 line
Modified regular file file1:
1 1: aA
2 2: b
3 3: cC
4 4: d
=== No gap to skip
Modified regular file file1:
1 1: a
2 2: bB
3 3: cC
4 4: d
=== 1 line at start
Modified regular file file1:
1 1: a
2 2: bB
...
=== 1 line at end
Modified regular file file1:
...
3 3: cC
4 4: d
"###);
}

#[test]
fn test_diff_external_tool() {
let mut test_env = TestEnvironment::default();
Expand Down

0 comments on commit bedea06

Please sign in to comment.