rn-134: add line log pipeline article

Author: chriscool

rev_news/drafts/edition-134.md

@@ -21,9 +21,190 @@ This edition covers what happened during the months of March and April 2026.
 ### General
 -->
 
-<!---
 ### Reviews
--->
+
++ [[PATCH 0/4] line-log: route -L output through the standard diff pipeline](https://lore.kernel.org/git/pull.2065.git.1772845338.gitgitgadget@gmail.com)
+
+  `git log -L` lets users follow the history of a specified line range
+  inside a file, for example by passing `-L:funcname:file.c` to track
+  the evolution of a function. Since the feature was introduced,
+  however, its diff output has been generated by a hand-rolled helper
+  called `dump_diff_hacky()` rather than by Git's standard diff
+  pipeline, with a `NEEDSWORK` comment in `line-log.c` openly
+  admitting:
+
+  ```
+  /*
+   * NEEDSWORK: manually building a diff here is not the Right
+   * Thing(tm).  log -L should be built into the diff pipeline.
+   */
+  ```
+
+  The practical consequence is that almost every diff formatting
+  option that users have come to rely on (`--word-diff`,
+  `--color-moved`, the `-w`/`-b` whitespace options, `--no-prefix`,
+  `--src-prefix`/`--dst-prefix`, `--full-index`, `--abbrev`, `-R`,
+  `--output-indicator-*`, the pickaxe options `-S`/`-G`, and so on) is
+  silently ignored when combined with `-L`. The hand-rolled output
+  also omits the `index` lines, `new file mode` headers, and funcname
+  context in `@@` hunk headers that the standard pipeline produces.
+
+  Michael Montalbo opened the discussion by sending a four-patch
+  series that finally addresses this long-standing limitation. The
+  series explicitly replaced an earlier attempt of his,
+  ["line-log: fix `-L` with pickaxe options"](https://lore.kernel.org/git/pull.2061.git.1772651484.gitgitgadget@gmail.com/),
+  which had taken the opposite approach of *rejecting* `-S`/`-G` when
+  combined with `-L`; the new direction is to make those options
+  *work* instead. Patch 1 carries over a crash fix from that previous
+  attempt unchanged. Patch 2 is the core change. Patch 3 adds an
+  extensive set of tests for the newly-working options. Patch 4
+  updates the documentation.
+
+  Patch 1 fixes a real assertion failure that could be triggered by
+  combining `-L` with pickaxe options across a merge that contains a
+  rename, an issue originally reported by Matthew Hughes. Inside
+  `queue_diffs()`, the caller's `diff_options` was being reused for
+  rename detection, which meant that any user-specified pickaxe state
+  (`-G`, `-S`, or `--find-object`) would run inside `diffcore_std()`
+  and silently discard diff pairs that the rename machinery still
+  needed. The fix builds a private `diff_options` for the
+  rename-detection path, mirroring the pattern already used in `git
+  blame`'s `find_rename()`, and isolates the rename machinery from
+  unrelated user options.
+
+  Patch 2 is where the heavy lifting happens. Instead of formatting
+  output by hand, `-L` now feeds its filepairs through
+  `builtin_diff()` and `fn_out_consume()`, the same path used by
+  `git diff` and `git log -p`. The mechanism is a pair of callback
+  wrappers that sit between `xdi_diff_outf()` and `fn_out_consume()`,
+  filtering xdiff's output down to only the tracked line ranges. To
+  make sure xdiff actually emits every line within each tracked range
+  as context, the context length is inflated to span the largest
+  range. The tracked line ranges themselves are now carried on `struct
+  diff_filepair` as a borrowed pointer, so that each file's ranges
+  travel with its filepair through the rest of the pipeline. As a side
+  effect, `line_log_print()` shrinks down to little more than a
+  `diffcore_std()` call followed by `diff_flush()`, the
+  `-L`-implies-`--patch` default is wired up in revision setup rather
+  than forced at output time, and `diff_filepair_dup()` is switched
+  from `xmalloc` to `xcalloc` so that newly added fields (including
+  the `line_ranges`) are zero-initialized.
+
+  Because `diffcore_std()` now actually runs at output time, options
+  such as `-S`, `-G`, `--orderfile`, and `--diff-filter` come along
+  for the ride and start working with `-L` for the first time. Michael
+  also notes in the commit message that the context-length inflation
+  means xdiff might process more output than strictly needed for very
+  wide ranges, but his benchmarks on files up to 7800 lines showed no
+  measurable regression.
+
+  There is, of course, a user-visible output change: `-L` output now
+  includes `index` lines, `new file mode` headers, and funcname
+  context in `@@` hunk headers that were previously absent. Tools that
+  parse `-L` output may need to handle these additional lines. The
+  cover letter is upfront about this, and also lists two limitations
+  that are deliberately left for follow-up work: `line_log_print()`
+  still calls `show_log()` and `diff_flush()` directly rather than
+  going through `log_tree_diff_flush()`, and the non-patch diff
+  formats (`--raw`, `--numstat`, `--stat`, etc.) remain unimplemented
+  for `-L`.
+
+  Junio Hamano, the Git maintainer, replied to the cover letter the
+  same day with a single word: "Exciting." He approved the deliberate
+  incremental scope, observing that since "previously all the output
+  routines were hand-rolled, but this reduces the extent of deviation
+  --- as long as we are moving in the right direction, it is a good
+  idea to find a good place to stop and leave the rest for later." On
+  the note about non-patch diff formats, Junio remarked that "it would
+  not hurt if these are omitted", which led to a small back-and-forth
+  where Michael initially thought he was being asked to do something
+  extra in a follow-up; Junio clarified that the series was already
+  omitting them ("You are already omitting, no? I took 'remain
+  unimplemented' to mean exactly that"), and that simply *mentioning*
+  the omission, as the cover letter already did, was the right thing
+  to do.
+
+  Junio also pointed out that the "Michael Montalbo (4): ..." block in
+  the cover letter looked like a reflowed duplicate of the proper
+  commit list right below it. Michael acknowledged that as a mistake
+  in crafting the cover letter and offered to add a few names from
+  `git shortlog --no-merges -s -n line-log.[ch]` to the Cc list to
+  attract more reviewers.
+
+  On the documentation patch (4/4), Kristoffer Haugsbakk caught a
+  subtle AsciiDoc problem: by indenting the new paragraph with tabs,
+  Michael had inadvertently turned the new prose into a code block.
+  Kristoffer recommended dropping the indentation in favour of a plain
+  list-continuation marker so the text would render as regular
+  paragraph text, "flush to the left." Michael thanked him and folded
+  the fix into his next iteration.
+
+  For readers less familiar with the relevant pieces of the diff
+  stack, a few words of context may help.
+
+  `git log -L` is itself a relatively unusual citizen in Git's command
+  zoo: most `git log` machinery walks commits and emits whatever its
+  configured formatters dictate, but `-L` additionally carries a set
+  of line ranges per file, narrowing the history to commits that touch
+  those ranges. Mapping that range-aware view onto the standard diff
+  output machinery is non-trivial because xdiff itself does not know
+  anything about the user's tracked ranges; it just produces a unified
+  diff for two blobs. The new callback wrappers introduced in patch 2
+  bridge that gap by intercepting xdiff's output as it is generated
+  and discarding hunks that fall outside the requested ranges.
+
+  The `diffcore_std()` function is the standard point at which Git
+  applies a number of cross-cutting transformations to a queued set of
+  diff pairs: rename detection, the pickaxe filters (`-S`, `-G`,
+  `--find-object`), the orderfile sort, and the `--diff-filter`
+  filter, among others. Once `-L` actually feeds its pairs through
+  this function, all of those features become available essentially
+  "for free." That is also why patch 1 has to be careful: rename
+  detection performed during the line-history walk must *not* let a
+  user's pickaxe filter inadvertently throw away the very pairs the
+  rename machinery needs to do its job.
+
+  After the initial round of review, Michael sent
+  [version 2](https://lore.kernel.org/git/pull.2065.v2.git.1773714095.gitgitgadget@gmail.com)
+  of the series. The only structural change from v1 was that patch 4
+  now uses a list-continuation marker instead of indentation in
+  `Documentation/line-range-options.adoc`, addressing Kristoffer's
+  review feedback so the new paragraph renders correctly. The
+  crash-fix patch (1/4) also gained an explanatory comment in its test
+  file about commit-level filtering with pickaxe still being a known
+  limitation: `show_log()` prints the commit header before
+  `diffcore_std()` runs, so commits cannot yet be suppressed even when
+  no diff pairs survive filtering. Fixing that would require deferring
+  `show_log()` until after `diffcore_std()`, which is again a larger
+  log-tree restructuring that v2 explicitly leaves for later.
+
+  Junio reviewed v2 patch 2 again and was generally positive. He noted
+  that "huge diff to the test material mostly comes from the addition
+  of the diff headers like the index line, etc., which makes this
+  patch scary but is very welcome addition", and on the new field
+  `line_ranges` carried on `struct diff_filepair`, simply replied
+  "OK." On the rewrite of `line_log_print()` itself, which now queues
+  a duplicated filepair per range, attaches the borrowed
+  `line_ranges`, and calls `diffcore_std()` followed by
+  `diff_flush()`, he wrote: "Very welcome change."
+
+  Some weeks later, after no further substantive review arrived, Junio
+  came back to the v2 cover letter and wrote, in a slightly resigned
+  but encouraging tone: "The central part of the series (i.e., patch
+  #2) looked quite sensible. I haven't read the tests very carefully,
+  though. I was hoping that we will see another set of eyes or two to
+  help review this series, but nothing has happened in the past few
+  weeks, so let's mark the topic for 'next'." The series was later
+  merged into 'master' and these improvements have been released as
+  part of Git v2.54.0.
+
+  This is an example of long-standing technical debt finally being
+  paid down. A `NEEDSWORK` comment that has lived in `line-log.c` for
+  many years is finally retired; an entire family of diff options
+  (formatting, whitespace, pickaxe, output-indicator, prefix,
+  color-moved, and more) becomes available with `-L` for the first
+  time; and a real assertion failure involving merges, renames, and
+  pickaxe filters is fixed along the way.
 
 <!---
 ### Support