Re: [PATCH v7 0/5] git log -L, all new and shiny

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

Thomas Rast <trast@student.ethz.ch> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Thomas Rast <trast@student.ethz.ch> writes:
>>
>>> I too thought it would never happen -- but then again this is still
>>> not ready, I'm just trying to give it some exposure.
>>> ...
>>> There's also a longer-term wishlist hinted at in the commit message of
>>> the main patch: the diff machinery currently makes no provisions for
>>> chaining its various bells and whistles.
>>
>> I am not convinced that it is "diff machinery makes no provivsions"
>> that is the problem. Isn't it coming from the way the series limits
>> the output line range and reimplements its own output routine?
>
> Well, in a very circular logic sense, yes: I reimplement the output
> routine because that's the only way I could think of doing it right now :-)
>
> However, notice that word-diff also reimplements its own output routine,
> though it probably has a better standing since it is a different format.

Also notice that word-diff uses the same xdi_diff_outf() machinery
to grab line-oriented diff as its input (cf. fn_out_consume), and
then does its thing on it.  If you limit what fn_out_consume sees,
you can have word-diff do exactly what you want, no?

> This would be the first backwards coupling between the revision-walk and
> the diff generation parts, at least that I know of.

I am not convinced if you need to have any unusual back-coupling to
begin with, by the way.

If you say "git log -p [--options] -- pathspec", the revision
machinery does filter commits that do not touch any paths that patch
pathspec with the TREESAME logic, but that does not necessarily mean
you will see _all_ the commits that are not TREESAME.  If you for
example use ignore-space-change options, even the preimage and the
postimage differ at the object name level (hence not TREESAME), the
diff machinery already knows how to tell the revision machinery not
to show the log message and stuff, causing the commit to be skipped
from the output, no?

I do not know why you think you would need to do the filtering
"range comparison and union" computation more than necessary.  If
the user asks "log -p", you need to do it once per parent-child pair
that is not TREESAME at the place the current code calls run_diff().
I suspect that "log -p --stat" could be improved to eliminate the
separate call to run_diffstat() by restructuring the code so that
the statistics is gathered inside run_diff(), but that is independent
of this series. If this series hooked into the level I hinted in my
earlier message, such an optimization will reduce calls to your
"range comparision and union" computation for free.