Skip to content

line-log: route -L output through the standard diff pipeline

Michael Montalbo via GitGitGadget git 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest4 patches 2 repliesIn review
  2. v14 patches 6 repliesSuperseded

Changes in v2 from the cover letter

* Patch 4/4: fix documentation formatting: use line continuation instead of indentation (Kristoffer Haugsbakk)

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/4line-log: fix crash when combined with pickaxe optionsline-log.c t/t4211-line-log.sh+73 -40 replies
2/4line-log: route -L output through the standard diff pipelinediff.c diffcore.h line-log.c +31 more+512 -2131 reply
3/4t4211: add tests for -L with standard diff optionst/t4211-line-log.sh+281 -00 replies
4/4doc: note that -L supports patch formatting and pickaxe optionsDocumentation/line-range-options.adoc+4 -00 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.

Read the v2 thread

Cover letter [PATCH v2 0/4] line-log: route -L output through the standard diff pipeline

git log -L has bypassed the standard diff pipeline since its introduction, using dump_diff_hacky() to hand-roll diff output. A NEEDSWORK comment has acknowledged this from the start. This series removes dump_diff_hacky() and routes -L output through builtin_diff() / fn_out_consume(), so that diff formatting options like --word-diff, --color-moved, -w, and pickaxe options (-S, -G) work with -L.

This replaces my earlier series "line-log: fix -L with pickaxe options" [1]. Patch 1 is the crash fix from that series (unchanged). Patch 2/2 from that series (rejecting -S/-G) is dropped because this series makes those options work instead of rejecting them.

[1]
https://lore.kernel.org/git/[email protected]/

Patch 1 fixes a crash when combining -L with pickaxe options and a rename.

Patch 2 is the core change: callback wrappers filter xdiff's output to tracked line ranges, and line ranges are carried on diff_filepair so each file's ranges travel with its filepair through the pipeline. diffcore_std() runs at output time, so pickaxe, --orderfile, and --diff-filter also work.

Patch 3 adds tests covering the newly-working options.

Patch 4 updates documentation.

User-visible output change: -L output now includes index lines, new file mode headers, and funcname context in @@ headers that were previously missing. Tools parsing -L output may need to handle these additional lines.

Known limitations not addressed in this series:

* line_log_print() still calls show_log() and diff_flush() directly, bypassing log_tree_diff_flush(). The early return in log_tree_commit() (and its associated NEEDSWORK about no_free not being restored) is pre-existing. Restructuring -L to flow through log_tree_diff_flush() is a larger change that would affect separator and header logic; it is left for a follow-up.

* Non-patch diff formats (--raw, --numstat, --stat, etc.) remain unimplemented for -L.

cc "Kristoffer Haugsbakk" [email protected]

Changes since v1:

* Patch 4/4: fix documentation formatting: use line continuation instead of indentation (Kristoffer Haugsbakk)

Michael Montalbo (4):
line-log: fix crash when combined with pickaxe options
line-log: route -L output through the standard diff pipeline
t4211: add tests for -L with standard diff options
doc: note that -L supports patch formatting and pickaxe options

 Documentation/line-range-options.adoc         |   4 +
 diff.c                                        | 279 +++++++++++++-
 diffcore.h                                    |  16 +
 line-log.c                                    | 196 ++--------
 line-log.h                                    |  14 +-
 revision.c                                    |   2 +
 t/t4211-line-log.sh                           | 348 +++++++++++++++++-
 t/t4211/sha1/expect.beginning-of-file         |   4 +
 t/t4211/sha1/expect.end-of-file               |  11 +-
 t/t4211/sha1/expect.move-support-f            |   5 +
 t/t4211/sha1/expect.multiple                  |  10 +-
 t/t4211/sha1/expect.multiple-overlapping      |   7 +
 t/t4211/sha1/expect.multiple-superset         |   7 +
 t/t4211/sha1/expect.no-assertion-error        |  12 +-
 t/t4211/sha1/expect.parallel-change-f-to-main |   7 +
 t/t4211/sha1/expect.simple-f                  |   4 +
 t/t4211/sha1/expect.simple-f-to-main          |   5 +
 t/t4211/sha1/expect.simple-main               |  11 +-
 t/t4211/sha1/expect.simple-main-to-end        |  11 +-
 t/t4211/sha1/expect.two-ranges                |  10 +-
 t/t4211/sha1/expect.vanishes-early            |  10 +-
 t/t4211/sha256/expect.beginning-of-file       |   4 +
 t/t4211/sha256/expect.end-of-file             |  11 +-
 t/t4211/sha256/expect.move-support-f          |   5 +
 t/t4211/sha256/expect.multiple                |  10 +-
 t/t4211/sha256/expect.multiple-overlapping    |   7 +
 t/t4211/sha256/expect.multiple-superset       |   7 +
 t/t4211/sha256/expect.no-assertion-error      |  12 +-
 .../sha256/expect.parallel-change-f-to-main   |   7 +
 t/t4211/sha256/expect.simple-f                |   4 +
 t/t4211/sha256/expect.simple-f-to-main        |   5 +
 t/t4211/sha256/expect.simple-main             |  11 +-
 t/t4211/sha256/expect.simple-main-to-end      |  11 +-
 t/t4211/sha256/expect.two-ranges              |  10 +-
 t/t4211/sha256/expect.vanishes-early          |  10 +-
 35 files changed, 870 insertions(+), 217 deletions(-)

base-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2065%2Fmmontalbo%2Fspike-xdiff-line-range-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2065/mmontalbo/spike-xdiff-line-range-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2065

Range-diff vs v1:

1: e7b8cc2c78 ! 1: ccfc1b03ff line-log: fix crash when combined with pickaxe options
@@ t/t4211-line-log.sh: test_expect_success 'show line-log with graph' '
+ --find-object=$(git rev-parse HEAD:file) >actual
+'
+
++# Commit-level filtering with pickaxe does not yet work for -L. ++# show_log() prints the commit header before diffcore_std() runs ++# pickaxe, so commits cannot be suppressed even when no diff pairs ++# survive filtering. Fixing this would require deferring show_log() ++# until after diffcore_std(), which is a larger restructuring of the ++# log-tree output pipeline.
+test_expect_failure '-L -G should filter commits by pattern' '
+ git log --format="%s" --no-patch -L 1,1:file -G "nomatch" >actual &&
+ test_must_be_empty actual

 2:  6921b07a0f = 2:  4e2bc55082 line-log: route -L output through the standard diff pipeline
 3:  09e00d3f82 = 3:  cf7720ae98 t4211: add tests for -L with standard diff options
 4:  0d3e84a088 ! 4:  93e771b7cd doc: note that -L supports patch formatting and pickaxe options
     @@ Commit message
      
       ## Documentation/line-range-options.adoc ##
      @@
     - 	Patch output can be suppressed using `--no-patch`, but other diff formats
       	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
       	`--name-only`, `--name-status`, `--check`) are not currently implemented.
     -++
     -+	Patch formatting options such as `--word-diff`, `--color-moved`,
     -+	`--no-prefix`, and whitespace options (`-w`, `-b`) are supported,
     -+	as are pickaxe options (`-S`, `-G`).
       +
     ++Patch formatting options such as `--word-diff`, `--color-moved`,
     ++`--no-prefix`, and whitespace options (`-w`, `-b`) are supported,
     ++as are pickaxe options (`-S`, `-G`).
     +++
       include::line-range-format.adoc[]
Signature · 1 line
gitgitgadget