[PATCH 2 of 3] context: add a `blockancestors(fromline, toline)` method to filectx

Denis Laxalde denis.laxalde at logilab.fr
Fri Dec 2 02:36:42 EST 2016


Yuya Nishihara a écrit :
> On Mon, 28 Nov 2016 10:54:15 +0100, Denis Laxalde wrote:
>> # HG changeset patch
>> # User Denis Laxalde <denis at laxalde.org>
>> # Date 1480086869 -3600
>> #      Fri Nov 25 16:14:29 2016 +0100
>> # Node ID 6dd93ae7b35002531308444c87dcf47beb773648
>> # Parent  0cf70234a38e47a3f7107611885368db9d52f574
>> # EXP-Topic linerange-log/revset
>> context: add a `blockancestors(fromline, toline)` method to filectx
>>
>> This yields filectx instances obtained by walking through ancestors by
>> only keeping changesets touching the file within specified `linerange =
>> (fromline, toline)`. It only follows first parent, since this yields weird
>> results in case of merge (might be improved later).
>>
>> Matching revisions are found by inspecting the result of `mdiff.allblocks()`,
>> filtered by `mdiff.blocksinrange()`, to find out if there are blocks of type
>> "!" within specified line range.
>>
>> diff --git a/mercurial/context.py b/mercurial/context.py
>> --- a/mercurial/context.py
>> +++ b/mercurial/context.py
>> @@ -1153,6 +1153,40 @@ class filectx(basefilectx):
>>          return [filectx(self._repo, self._path, fileid=x,
>>                          filelog=self._filelog) for x in c]
>>
>> +    def blockancestors(self, fromline, toline):
>> +        """Yield ancestors of this filectx with respect to the block of lines
>> +        in `linerange = (lowerbound, upperbound)`.
>> +        """
>
> Nit: Perhaps this could be a module-level function since it doesn't depend on
> the internals of filectx.

Yes.

>> +        def changesrange(fctx1, fctx2, linerange2):
>> +            """Return `(diffinrange, linerange1)` where `diffinrange` is True
>> +            if diff from fctx2 to fctx1 has changes in linerange2 and
>> +            `linerange1` is the new line range for fctx1.
>> +            """
>> +            diffopts = patch.diffopts(self._repo.ui)
>
> Is it intended to take all diff options such as ignorews?

I'd say so, at least when used from the "changes" revset. Perhaps the
function could get an opts keyword argument and defaults to this (or
mdiff.defaultopts maybe) if unspecified. Do you see any case where this
wouldn't be desirable?

>> +            blocks = mdiff.allblocks(fctx1.data(), fctx2.data(), diffopts)
>> +            blocks = mdiff.blocksinrange(blocks, linerange2)
>> +            # consume blocks to find out linerange1 and whether there's some
>> +            # diff in linerange2.
>> +            diffinrange = False
>> +            while True:
>> +                try:
>> +                    _, stype = next(blocks)
>> +                    if diffinrange:
>> +                        continue
>> +                    diffinrange = stype == '!'
>> +                except StopIteration as exc:
>> +                    linerange1 = exc.args[0]
>> +                    break
>> +            return diffinrange, linerange1
>> +
>> +        c, linerange2 = self, (fromline, toline)
>> +        for p in self.ancestors(followfirst=True):  # TODO handle followfirst
>> +            inrangep, linerange1 = changesrange(p, c, linerange2)
>> +            if inrangep:
>> +                yield c
>> +            c, linerange2 = p, linerange1
>
>> +        yield c  # XXX should we?
>
> Maybe yes, considering there's a diff between null and c.
>

Yes, that was my feeling also. Thanks for confirming.


More information about the Mercurial-devel mailing list