From bedea0699ebf61ce36c9fb3d97795c17b55cc02e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anton=20=C3=84lgmyr?= Date: Sat, 2 Mar 2024 01:57:43 +0100 Subject: [PATCH] Add --context flag for diffs. Allows specifying the number of lines of context to show around diffs. The logic was already in place, just some plumbing was needed. --- CHANGELOG.md | 3 + cli/src/diff_util.rs | 81 ++++++++++++++++++------ cli/tests/cli-reference@.md.snap | 5 ++ cli/tests/test_diff_command.rs | 103 +++++++++++++++++++++++++++++++ 4 files changed, 172 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1a6b98ef5..e1e66feb6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index 6f52ad294a..0c04977404 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -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"])))] @@ -73,6 +75,9 @@ pub struct DiffFormatArgs { /// Generate diff by external command #[arg(long)] pub tool: Option, + /// Number of lines of context to show + #[arg(long)] + context: Option, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -80,8 +85,8 @@ pub enum DiffFormat { Summary, Stat, Types, - Git, - ColorWords, + Git { context: usize }, + ColorWords { context: usize }, Tool(Box), } @@ -92,7 +97,7 @@ pub fn diff_formats_for( ) -> Result, 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) } @@ -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) @@ -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() @@ -136,7 +151,10 @@ fn diff_formats_from_args( Ok(formats) } -fn default_diff_format(settings: &UserSettings) -> Result { +fn default_diff_format( + settings: &UserSettings, + num_context_lines: Option, +) -> Result { let config = settings.config(); if let Some(args) = config.get("ui.diff.tool").optional()? { // External "tool" overrides the internal "format" option. @@ -158,8 +176,12 @@ fn default_diff_format(settings: &UserSettings) -> Result 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}" @@ -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)?; @@ -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; @@ -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")?; @@ -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) { @@ -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, )?; } @@ -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, + )?; } } } @@ -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"), "@@ -{},{} +{},{} @@", @@ -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")?; @@ -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)?; @@ -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| { @@ -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>(()) diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 19a23e5bef..6a7d183c26 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -655,6 +655,7 @@ With the `--from` and/or `--to` options, shows the difference from/to the given Possible values: `true`, `false` * `--tool ` — Generate diff by external command +* `--context ` — Number of lines of context to show @@ -1000,6 +1001,7 @@ This excludes changes from other commits by temporarily rebasing `--from` onto ` Possible values: `true`, `false` * `--tool ` — Generate diff by external command +* `--context ` — Number of lines of context to show @@ -1051,6 +1053,7 @@ Show commit history Possible values: `true`, `false` * `--tool ` — Generate diff by external command +* `--context ` — Number of lines of context to show @@ -1221,6 +1224,7 @@ Show how a change has evolved as it's been updated, rebased, etc. Possible values: `true`, `false` * `--tool ` — Generate diff by external command +* `--context ` — Number of lines of context to show @@ -1592,6 +1596,7 @@ Show commit description and changes in a revision Possible values: `true`, `false` * `--tool ` — Generate diff by external command +* `--context ` — Number of lines of context to show diff --git a/cli/tests/test_diff_command.rs b/cli/tests/test_diff_command.rs index c4405ceb01..ce685df211 100644 --- a/cli/tests/test_diff_command.rs +++ b/cli/tests/test_diff_command.rs @@ -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 @@ -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 @@ -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();