git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 2/2] log: reject pickaxe options when combined with -L

From
Michael Montalbo <mmontalbo@gmail.com>
Date
Mar 4, 2026, 22:36 UTC
Message-ID
<CAC2Qwm+2pjMk=XFq0cU0Pt1tWkqDy_tOKMtP0jF6JArFX0jmOg@mail.gmail.com>
In-Reply-To
<xmqq4imv71g8.fsf@gitster.g>
On Wed, Mar 4, 2026 at 1:02 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 39 quoted lines
>
> "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Michael Montalbo <mmontalbo@gmail.com>
> >
> > The previous commit fixed a crash when -G, -S, or --find-object was
> > used together with -L and rename detection.  However, these options
> > still have no effect on -L output: line-log uses its own
> > commit-filtering logic in line_log_filter() and never consults the
> > pickaxe machinery.  Rather than silently ignoring these options, reject
> > the combination with a clear error message.
> >
> > This replaces the known-breakage tests from the previous commit with
> > tests that verify the rejection for all three options.  A future series
> > could teach line-log to honor these options and remove this restriction.
> >
> > Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
> > ---
> >  builtin/log.c       |  4 ++++
> >  t/t4211-line-log.sh | 52 ++++++++-------------------------------------
> >  2 files changed, 13 insertions(+), 43 deletions(-)
> >
> > diff --git a/builtin/log.c b/builtin/log.c
> > index 5c9a8ef363..44e2399d59 100644
> > --- a/builtin/log.c
> > +++ b/builtin/log.c
> > @@ -317,6 +317,10 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
> >       if (rev->line_level_traverse && rev->prune_data.nr)
> >               die(_("-L<range>:<file> cannot be used with pathspec"));
> >
> > +     if (rev->line_level_traverse &&
> > +         (rev->diffopt.pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))
> > +             die(_("-L does not yet support -G, -S, or --find-object"));
>
> I do not think "-L" meant to work well with these features to begin
> with, and I've never used -L with any other options (-L does not
> even work with --stat), so I personally do not mind this change.
>
> But if this is in place, would we still need [1/2]?

I went back and forth on whether to keep [1/2]. My main reason for keeping it was as future-proofing if someone removes the die() to implement support for these features working together.

However, I can easily see the argument that whoever does that work would likely rework queue_diffs() anyway and it's simpler to drop it. Happy to do so if it's not worth the churn.

Previous: Junio C Hamano
Message 10 of 10 in “line-log: fix -L with pickaxe options”
  1. 0/2 line-log: fix -L with pickaxe optionsMichael Montalbo via GitGitGadget, Mar 4, 2026
  2. 1/2 line-log: fix crash when combined with pickaxe optionsMichael Montalbo via GitGitGadget, Mar 4, 2026
  3. Junio C HamanoMar 4, 2026
  4. Michael MontalboMar 4, 2026
  5. 2/2 log: reject pickaxe options when combined with -LMichael Montalbo via GitGitGadget, Mar 4, 2026
  6. 0/2 line-log: fix -L with pickaxe optionsMichael Montalbo via GitGitGadget, Mar 4, 2026
  7. 1/2 line-log: fix crash when combined with pickaxe optionsMichael Montalbo via GitGitGadget, Mar 4, 2026
  8. 2/2 log: reject pickaxe options when combined with -LMichael Montalbo via GitGitGadget, Mar 4, 2026
  9. Junio C HamanoMar 4, 2026
  10. Michael MontalboMar 4, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.