cli: add diff --ignore-all-space/--ignore-space-change flags

These flags only apply to line-based diffs. This is easy, and seems still useful
to highlight whitespace changes (that could be ignored by line diffing.)

I've added short options only to "diff"-like commands. It seemed unclear if
they were added to deeply-nested commands such as "op log".

Closes #3781

CHANGELOG.md

@@ -17,6 +17,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### New features
 
+* Added diff options to ignore whitespace when comparing lines. Whitespace
+  changes are still highlighted.
+
 ### Fixed bugs
 
 

cli/src/commands/diff.rs

@@ -38,6 +38,8 @@ use crate::ui::Ui;
 /// commit. For example, `jj diff --from main` shows the changes from "main"
 /// (perhaps a bookmark name) to the working-copy commit.
 #[derive(clap::Args, Clone, Debug)]
+#[command(mut_arg("ignore_all_space", |a| a.short('w')))]
+#[command(mut_arg("ignore_space_change", |a| a.short('b')))]
 pub(crate) struct DiffArgs {
     /// Show changes in this revision, compared to its parent(s)
     ///

cli/src/commands/interdiff.rs

@@ -30,6 +30,8 @@ use crate::ui::Ui;
 /// versions, consider `jj evolog -p` instead.
 #[derive(clap::Args, Clone, Debug)]
 #[command(group(ArgGroup::new("to_diff").args(&["from", "to"]).multiple(true).required(true)))]
+#[command(mut_arg("ignore_all_space", |a| a.short('w')))]
+#[command(mut_arg("ignore_space_change", |a| a.short('b')))]
 pub(crate) struct InterdiffArgs {
     /// Show changes from this revision
     #[arg(long)]

cli/src/commit_templater.rs

@@ -1506,6 +1506,9 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T
                     // TODO: load defaults from UserSettings?
                     let options = diff_util::ColorWordsDiffOptions {
                         context: context.unwrap_or(diff_util::DEFAULT_CONTEXT_LINES),
+                        line_diff: diff_util::LineDiffOptions {
+                            compare_mode: diff_util::LineCompareMode::Exact,
+                        },
                         max_inline_alternation: Some(3),
                     };
                     diff.into_formatted(move |formatter, store, tree_diff| {
@@ -1540,6 +1543,9 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T
                 .map(|(diff, context)| {
                     let options = diff_util::UnifiedDiffOptions {
                         context: context.unwrap_or(diff_util::DEFAULT_CONTEXT_LINES),
+                        line_diff: diff_util::LineDiffOptions {
+                            compare_mode: diff_util::LineCompareMode::Exact,
+                        },
                     };
                     diff.into_formatted(move |formatter, store, tree_diff| {
                         diff_util::show_git_diff(formatter, store, tree_diff, &options)
@@ -1562,7 +1568,11 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T
             let path_converter = language.path_converter;
             let template = (self_property, width_property)
                 .map(move |(diff, width)| {
-                    let options = diff_util::DiffStatOptions {};
+                    let options = diff_util::DiffStatOptions {
+                        line_diff: diff_util::LineDiffOptions {
+                            compare_mode: diff_util::LineCompareMode::Exact,
+                        },
+                    };
                     diff.into_formatted(move |formatter, store, tree_diff| {
                         diff_util::show_diff_stat(
                             formatter,

cli/src/diff_util.rs

@@ -39,6 +39,10 @@ use jj_lib::conflicts::MaterializedTreeValue;
 use jj_lib::copies::CopiesTreeDiffEntry;
 use jj_lib::copies::CopyOperation;
 use jj_lib::copies::CopyRecords;
+use jj_lib::diff::find_line_ranges;
+use jj_lib::diff::CompareBytesExactly;
+use jj_lib::diff::CompareBytesIgnoreAllWhitespace;
+use jj_lib::diff::CompareBytesIgnoreWhitespaceAmount;
 use jj_lib::diff::Diff;
 use jj_lib::diff::DiffHunk;
 use jj_lib::diff::DiffHunkKind;
@@ -113,6 +117,14 @@ pub struct DiffFormatArgs {
     /// Number of lines of context to show
     #[arg(long)]
     context: Option<usize>,
+
+    // Short flags are set by command to avoid future conflicts.
+    /// Ignore whitespace when comparing lines.
+    #[arg(long)] // short = 'w'
+    ignore_all_space: bool,
+    /// Ignore changes in amount of whitespace when comparing lines.
+    #[arg(long, conflicts_with = "ignore_all_space")] // short = 'b'
+    ignore_space_change: bool,
 }
 
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -430,10 +442,63 @@ pub fn get_copy_records<'a>(
     Ok(block_on_stream(stream).filter_ok(|record| matcher.matches(&record.target)))
 }
 
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct LineDiffOptions {
+    /// How equivalence of lines is tested.
+    pub compare_mode: LineCompareMode,
+    // TODO: add --ignore-blank-lines, etc. which aren't mutually exclusive.
+}
+
+impl LineDiffOptions {
+    fn from_args(args: &DiffFormatArgs) -> Self {
+        let compare_mode = if args.ignore_all_space {
+            LineCompareMode::IgnoreAllSpace
+        } else if args.ignore_space_change {
+            LineCompareMode::IgnoreSpaceChange
+        } else {
+            LineCompareMode::Exact
+        };
+        LineDiffOptions { compare_mode }
+    }
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum LineCompareMode {
+    /// Compares lines literally.
+    Exact,
+    /// Compares lines ignoring any whitespace occurrences.
+    IgnoreAllSpace,
+    /// Compares lines ignoring changes in whitespace amount.
+    IgnoreSpaceChange,
+}
+
+fn diff_by_line<'input, T: AsRef<[u8]> + ?Sized + 'input>(
+    inputs: impl IntoIterator<Item = &'input T>,
+    options: &LineDiffOptions,
+) -> Diff<'input> {
+    // TODO: If we add --ignore-blank-lines, its tokenizer will have to attach
+    // blank lines to the preceding range. Maybe it can also be implemented as a
+    // post-process (similar to refine_changed_regions()) that expands unchanged
+    // regions across blank lines.
+    match options.compare_mode {
+        LineCompareMode::Exact => {
+            Diff::for_tokenizer(inputs, find_line_ranges, CompareBytesExactly)
+        }
+        LineCompareMode::IgnoreAllSpace => {
+            Diff::for_tokenizer(inputs, find_line_ranges, CompareBytesIgnoreAllWhitespace)
+        }
+        LineCompareMode::IgnoreSpaceChange => {
+            Diff::for_tokenizer(inputs, find_line_ranges, CompareBytesIgnoreWhitespaceAmount)
+        }
+    }
+}
+
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct ColorWordsDiffOptions {
     /// Number of context lines to show.
     pub context: usize,
+    /// How lines are tokenized and compared.
+    pub line_diff: LineDiffOptions,
     /// Maximum number of removed/added word alternation to inline.
     pub max_inline_alternation: Option<usize>,
 }
@@ -455,6 +520,7 @@ impl ColorWordsDiffOptions {
         };
         Ok(ColorWordsDiffOptions {
             context: args.context.unwrap_or(DEFAULT_CONTEXT_LINES),
+            line_diff: LineDiffOptions::from_args(args),
             max_inline_alternation,
         })
     }
@@ -466,7 +532,7 @@ fn show_color_words_diff_hunks(
     right: &[u8],
     options: &ColorWordsDiffOptions,
 ) -> io::Result<()> {
-    let line_diff = Diff::by_line([left, right]);
+    let line_diff = diff_by_line([left, right], &options.line_diff);
     let mut line_number = DiffLineNumber { left: 1, right: 1 };
     // Matching entries shouldn't appear consecutively in diff of two inputs.
     // However, if the inputs have conflicts, there may be a hunk that can be
@@ -477,9 +543,13 @@ fn show_color_words_diff_hunks(
     for hunk in line_diff.hunks() {
         match hunk.kind {
             DiffHunkKind::Matching => {
-                // TODO: add support for unmatched contexts
-                debug_assert!(hunk.contents.iter().all_equal());
-                contexts.push(hunk.contents[0]);
+                // TODO: better handling of unmatched contexts
+                debug_assert!(hunk
+                    .contents
+                    .iter()
+                    .map(|content| content.split_inclusive(|b| *b == b'\n').count())
+                    .all_equal());
+                contexts.push(hunk.contents[1]);
             }
             DiffHunkKind::Different => {
                 let num_after = if emitted { options.context } else { 0 };
@@ -1097,12 +1167,15 @@ fn git_diff_part(
 pub struct UnifiedDiffOptions {
     /// Number of context lines to show.
     pub context: usize,
+    /// How lines are tokenized and compared.
+    pub line_diff: LineDiffOptions,
 }
 
 impl UnifiedDiffOptions {
     fn from_args(args: &DiffFormatArgs) -> Self {
         UnifiedDiffOptions {
             context: args.context.unwrap_or(DEFAULT_CONTEXT_LINES),
+            line_diff: LineDiffOptions::from_args(args),
         }
     }
 }
@@ -1165,14 +1238,16 @@ fn unified_diff_hunks<'content>(
         right_line_range: 1..1,
         lines: vec![],
     };
-    let diff = Diff::by_line([left_content, right_content]);
+    let diff = diff_by_line([left_content, right_content], &options.line_diff);
     let mut diff_hunks = diff.hunks().peekable();
     while let Some(hunk) = diff_hunks.next() {
         match hunk.kind {
             DiffHunkKind::Matching => {
-                // TODO: add support for unmatched contexts
-                debug_assert!(hunk.contents.iter().all_equal());
-                let mut lines = hunk.contents[0].split_inclusive(|b| *b == b'\n').fuse();
+                // Just use the right (i.e. new) content. We could count the
+                // number of skipped lines separately, but the number of the
+                // context lines should match the displayed content.
+                let [_, right] = hunk.contents.try_into().unwrap();
+                let mut lines = right.split_inclusive(|b| *b == b'\n').fuse();
                 if !current_hunk.lines.is_empty() {
                     // The previous hunk line should be either removed/added.
                     current_hunk.extend_context_lines(lines.by_ref().take(options.context));
@@ -1448,11 +1523,16 @@ pub fn show_diff_summary(
 }
 
 #[derive(Clone, Debug, Eq, PartialEq)]
-pub struct DiffStatOptions {}
+pub struct DiffStatOptions {
+    /// How lines are tokenized and compared.
+    pub line_diff: LineDiffOptions,
+}
 
 impl DiffStatOptions {
-    fn from_args(_args: &DiffFormatArgs) -> Self {
-        DiffStatOptions {}
+    fn from_args(args: &DiffFormatArgs) -> Self {
+        DiffStatOptions {
+            line_diff: LineDiffOptions::from_args(args),
+        }
     }
 }
 
@@ -1467,12 +1547,15 @@ fn get_diff_stat(
     path: String,
     left_content: &FileContent,
     right_content: &FileContent,
-    _options: &DiffStatOptions,
+    options: &DiffStatOptions,
 ) -> DiffStat {
     // TODO: this matches git's behavior, which is to count the number of newlines
     // in the file. but that behavior seems unhelpful; no one really cares how
     // many `0x0a` characters are in an image.
-    let diff = Diff::by_line([&left_content.contents, &right_content.contents]);
+    let diff = diff_by_line(
+        [&left_content.contents, &right_content.contents],
+        &options.line_diff,
+    );
     let mut added = 0;
     let mut removed = 0;
     for hunk in diff.hunks() {

cli/tests/cli-reference@.md.snap

@@ -641,6 +641,8 @@ With the `--from` and/or `--to` options, shows the difference from/to the given
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
 * `--context <CONTEXT>` — Number of lines of context to show
+* `-w`, `--ignore-all-space` — Ignore whitespace when comparing lines
+* `-b`, `--ignore-space-change` — Ignore changes in amount of whitespace when comparing lines
 
 
 
@@ -741,6 +743,8 @@ Lists the previous commits which a change has pointed to. The current commit of
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
 * `--context <CONTEXT>` — Number of lines of context to show
+* `--ignore-all-space` — Ignore whitespace when comparing lines
+* `--ignore-space-change` — Ignore changes in amount of whitespace when comparing lines
 
 
 
@@ -1202,6 +1206,8 @@ This excludes changes from other commits by temporarily rebasing `--from` onto `
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
 * `--context <CONTEXT>` — Number of lines of context to show
+* `-w`, `--ignore-all-space` — Ignore whitespace when comparing lines
+* `-b`, `--ignore-space-change` — Ignore changes in amount of whitespace when comparing lines
 
 
 
@@ -1245,6 +1251,8 @@ Spans of revisions that are not included in the graph per `--revisions` are rend
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
 * `--context <CONTEXT>` — Number of lines of context to show
+* `--ignore-all-space` — Ignore whitespace when comparing lines
+* `--ignore-space-change` — Ignore changes in amount of whitespace when comparing lines
 
 
 
@@ -1392,6 +1400,8 @@ Compare changes to the repository between two operations
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
 * `--context <CONTEXT>` — Number of lines of context to show
+* `--ignore-all-space` — Ignore whitespace when comparing lines
+* `--ignore-space-change` — Ignore changes in amount of whitespace when comparing lines
 
 
 
@@ -1426,6 +1436,8 @@ Like other commands, `jj op log` snapshots the current working-copy changes and
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
 * `--context <CONTEXT>` — Number of lines of context to show
+* `--ignore-all-space` — Ignore whitespace when comparing lines
+* `--ignore-space-change` — Ignore changes in amount of whitespace when comparing lines
 
 
 
@@ -1490,6 +1502,8 @@ Show changes to the repository in an operation
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
 * `--context <CONTEXT>` — Number of lines of context to show
+* `--ignore-all-space` — Ignore whitespace when comparing lines
+* `--ignore-space-change` — Ignore changes in amount of whitespace when comparing lines
 
 
 
@@ -1825,6 +1839,8 @@ Show commit description and changes in a revision
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
 * `--context <CONTEXT>` — Number of lines of context to show
+* `--ignore-all-space` — Ignore whitespace when comparing lines
+* `--ignore-space-change` — Ignore changes in amount of whitespace when comparing lines
 
 
 

cli/tests/test_diff_command.rs

@@ -1521,6 +1521,110 @@ fn test_color_words_diff_missing_newline() {
     "###);
 }
 
+#[test]
+fn test_diff_ignore_whitespace() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    std::fs::write(
+        repo_path.join("file1"),
+        indoc! {"
+            foo {
+                bar;
+            }
+            baz {}
+        "},
+    )
+    .unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["new", "-mindent + whitespace insertion"]);
+    std::fs::write(
+        repo_path.join("file1"),
+        indoc! {"
+            {
+                foo {
+                    bar;
+                }
+            }
+            baz {  }
+        "},
+    )
+    .unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["status"]);
+
+    // Git diff as reference output
+    let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "--ignore-all-space"]);
+    insta::assert_snapshot!(stdout, @r#"
+    diff --git a/file1 b/file1
+    index f532aa68ad..033c4a6168 100644
+    --- a/file1
+    +++ b/file1
+    @@ -1,4 +1,6 @@
+    +{
+         foo {
+             bar;
+         }
+    +}
+     baz {  }
+    "#);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "--ignore-space-change"]);
+    insta::assert_snapshot!(stdout, @r#"
+    diff --git a/file1 b/file1
+    index f532aa68ad..033c4a6168 100644
+    --- a/file1
+    +++ b/file1
+    @@ -1,4 +1,6 @@
+    -foo {
+    +{
+    +    foo {
+             bar;
+    +    }
+     }
+    -baz {}
+    +baz {  }
+    "#);
+
+    // Diff-stat should respects the whitespace options
+    let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat", "--ignore-all-space"]);
+    insta::assert_snapshot!(stdout, @r#"
+    file1 | 2 ++
+    1 file changed, 2 insertions(+), 0 deletions(-)
+    "#);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat", "--ignore-space-change"]);
+    insta::assert_snapshot!(stdout, @r#"
+    file1 | 6 ++++--
+    1 file changed, 4 insertions(+), 2 deletions(-)
+    "#);
+
+    // Word-level changes are still highlighted
+    let stdout = test_env.jj_cmd_success(
+        &repo_path,
+        &["diff", "--color=always", "--ignore-all-space"],
+    );
+    insta::assert_snapshot!(stdout, @r#"
+    Modified regular file file1:
+            1: {
+       1    2:     foo {
+       2    3:         bar;
+       3    4:     }
+            5: }
+       4    6: baz {  }
+    "#);
+    let stdout = test_env.jj_cmd_success(
+        &repo_path,
+        &["diff", "--color=always", "--ignore-space-change"],
+    );
+    insta::assert_snapshot!(stdout, @r#"
+    Modified regular file file1:
+            1: {
+       1    2:     foo {
+       2    3:         bar;
+            4:     }
+       3    5: }
+       4    6: baz {  }
+    "#);
+}
+
 #[test]
 fn test_diff_skipped_context() {
     let test_env = TestEnvironment::default();