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

From: Thomas Rast <trast@student.ethz.ch>
Date: Jun 15, 2012, 13:29 UTC
Message-ID: <8762as4sax.fsf@thomas.inf.ethz.ch>
In-Reply-To: <7vlijpchm2.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 12 quoted lines

> 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.

Show 10 quoted lines

>  - add a mechanism to pass the "interesting" line range and path
>    down to the callchain from xdi_diff_outf() to xdiff_outf();
>
>  - make one of these functions filter out (i.e. not call the
>    callback xdiff_emit_consume_fn) hunks that do not overlap with
>    the line range you are interested in (I would presume that they
>    would be a few new fields in xdemitconf_t structure); and
>
>  - while recording the corresponding line ranges in the other side
>    of the hunks that are output,

Hrm.

This would be the first backwards coupling between the revision-walk and the diff generation parts, at least that I know of. Normally the revision walker just calls out to the (line-wise, not tree-based) diff engine when it wants to show a commit. Now suddenly the diff engine is used (a lot, too) in simplifying the history.

Ideally we would want to reuse diffs that have already been generated, as this is a very expensive process. The current log -L implementation manages to do this at the cost of reimplementing the diff output routines instead.

You solve it instead by mandating that the diff engine itself updates the "interesting" ranges, but that needs a lot of inside knowledge: like in blame, we sometimes explore alternatives (e.g. for merges; or with -M, though log -L in this version does not implement that feature).

So we would end up with redoing diffs, or a very tight coupling, that IMHO just makes the mess worse.

Or am I missing something?

I instead have the vision that eventually diffs should be represented internally as something like my pairs of struct range_set. Then we could run more passes on them as needed, and have a "common currency" between all diff-related work. Only the last one should then actually output the diff.

That still doesn't properly account for the case where the data format is no longer in terms of hunks (such as for word-diff, or the stat formats), though.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch