[PATCH 3 of 3] revert: account for computed changes in interactive revert (issue5789)

Denis Laxalde denis at laxalde.org
Wed Feb 14 07:59:19 EST 2018


Yuya Nishihara a écrit :
> On Wed, 14 Feb 2018 13:41:18 +0100, Denis Laxalde wrote:
>> Yuya Nishihara a écrit :
>>> On Tue, 13 Feb 2018 21:46:54 +0100, Denis Laxalde wrote:
>>>> # HG changeset patch
>>>> # User Denis Laxalde <denis at laxalde.org>
>>>> # Date 1518554564 -3600
>>>> #      Tue Feb 13 21:42:44 2018 +0100
>>>> # Node ID a7d28fab177642e028e37b77727603601c3a76cb
>>>> # Parent  4b8c889eb9d0b7ca6883b93dbd476323c94f677f
>>>> # EXP-Topic revert-interactive-pats
>>>> revert: account for computed changes in interactive revert (issue5789)
>>>>
>>>> When going through _performrevert() in the interactive case, we build a
>>>> matcher with files to revert and pass it patch.diff() for later
>>>> selection of diff hunks to revert. The files set used to build the
>>>> matcher comes from dirstate and accounts for patterns explicitly passed
>>>> to revert ('hg revert -i <file>') and, in case of nonexistent pattern,
>>>> this set is empty (which is expected). Unfortunately, when going through
>>>> patch.diff() with the resulting matcher (for which .always() is True), a
>>>> new changes tuple will be built that completely ignores patterns passed
>>>> by the user. This leads to the situation described in issue5789, where
>>>> one gets prompted about reverting files unrelated to specified patterns
>>>> because they made a typo or so.
>>>>
>>>> We fix this by building a 'changes' tuple (scmutil.status tuple) from
>>>> information computed during dirstate inspection in cmdutil.revert() and
>>>> pass this to _performrevert() which then hands it to patch.diff(), thus
>>>> avoiding re-computation of a 'changes' tuple when 'match.always is True'.
>>>
>>> Perhaps we should instead build an exact matcher from a list of canonical
>>> paths:
>>>
>>>   torevert = actions['revert'][0]  # XXX needs to drop excluded_files
>>>   m = scmutil.matchfiles(repo, torevert)
>>>
>>
>> I actually tried that in the first place:
>>
>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> --- a/mercurial/cmdutil.py
>> +++ b/mercurial/cmdutil.py
>> @@ -2958,8 +2958,9 @@ def _performrevert(repo, parents, ctx, a
>>      newlyaddedandmodifiedfiles = set()
>>      if interactive:
>>          # Prompt the user for changes to revert
>> -        torevert = [repo.wjoin(f) for f in actions['revert'][0]]
>> -        m = scmutil.match(ctx, torevert, matcher_opts)
>> +        torevert = [repo.wjoin(f) for f in actions['revert'][0]
>> +                    if repo.wjoin(f) not in excluded_files]
>> +        m = scmutil.matchfiles(repo, torevert)
>>          diffopts = patch.difffeatureopts(repo.ui, whitespace=True)
>>          diffopts.nodates = True
>>          diffopts.git = True
>>
>>
>> but this makes many tests fail (in test-revert-interactive.t) and I did
>> not understand why.
> 
> matchfiles() requires canonical paths (i.e. slash-separated paths relative
> to repo.root), not filesystem paths. We'll need to drop wjoin().
> 

Ah, I didn't know that. Seems to work so sending a v2 soon. Thanks!


More information about the Mercurial-devel mailing list