Re: [PATCH 1/2] line-log: fix crash when combined with pickaxe options

[Junio C Hamano <gitster@pobox.com>]

"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Michael Montalbo <mmontalbo@gmail.com>
>
> queue_diffs() calls diffcore_std() to detect renames so that line-level
> history can follow files across renames.  When pickaxe options are
> present on the command line (-G and -S to filter by text pattern,
> --find-object to filter by object identity), diffcore_std() also runs
> diffcore_pickaxe(), which may discard diff pairs that are relevant for
> rename detection.  Losing those pairs breaks rename following.

Shouldn't that be solved not by omitting the necessary call to
diffcore_std(), but by using the "--pickaxe-all" option?

> Note that this only fixes the crash.  The -G, -S, and --find-object
> options still have no effect on -L output because line-log uses its
> own commit-filtering logic that bypasses the normal pickaxe pipeline.

I do not know exactly what -L really wants to do, but from the look
at a patch like this, it smells like it is abusing the diffcore
machinery.  If it wants to follow the rename history for individual
paths, even if the end-user's top-level command line option included
pickaxe or other fancy diffcore options, should it be *reusing* the
diff_options struct, prepared from the end-user request?  Shouldn't
it rather be using its own diffopt crafted for that rename tracking
purpose, I have to wonder.

Thanks.