Patchwork D7101: fix: match patterns relative to root

login
register
mail settings
Submitter phabricator
Date Oct. 14, 2019, 10:52 p.m.
Message ID <differential-rev-PHID-DREV-aqpfvhzba4kbqbmiqz53-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/42340/
State Superseded
Headers show

Comments

phabricator - Oct. 14, 2019, 10:52 p.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I was surprised fixer patterns (used to determine which fixers to run)
  are applies to the parent directory, not the repo root
  directory. Danny Hooper (the author of the extension) seemed to agree
  that it's better to apply them to the repo root, so that's what this
  patch does.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D7101

AFFECTED FILES
  hgext/fix.py
  tests/test-fix.t

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Oct. 15, 2019, 1:18 a.m.
This revision now requires changes to proceed.
mharbison72 added a comment.
mharbison72 requested changes to this revision.


  The part of the help text that documents `:pattern` and points to `hg help patterns` should probably explicitly state that they are all relative to repo root, regardless of what the latter says.
  
  I can’t tell from my phone, but should the matcher in `getworkqueue` and the status call in `pathstofix` also be adjusted?  (Not sure if there are others.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7101/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7101

To: martinvonz, #hg-reviewers, mharbison72
Cc: mharbison72, mercurial-devel
phabricator - Oct. 15, 2019, 3:54 a.m.
martinvonz added a comment.


  In D7101#104125 <https://phab.mercurial-scm.org/D7101#104125>, @mharbison72 wrote:
  
  > The part of the help text that documents `:pattern` and points to `hg help patterns` should probably explicitly state that they are all relative to repo root, regardless of what the latter says.
  
  Good point! Done.
  
  > I can’t tell from my phone, but should the matcher in `getworkqueue` and the status call in `pathstofix` also be adjusted?  (Not sure if there are others.)
  
  No, that matcher (it's the same instance that's passed between them) is created based on `pats` and `opts`, i.e. command line arguments.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7101/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7101

To: martinvonz, #hg-reviewers, mharbison72
Cc: mharbison72, mercurial-devel
phabricator - Oct. 15, 2019, 4:10 a.m.
mharbison72 added a comment.
mharbison72 accepted this revision.


  In D7101#104151 <https://phab.mercurial-scm.org/D7101#104151>, @martinvonz wrote:
  
  >> I can’t tell from my phone, but should the matcher in `getworkqueue` and the status call in `pathstofix` also be adjusted?  (Not sure if there are others.)
  >
  > No, that matcher (it's the same instance that's passed between them) is created based on `pats` and `opts`, i.e. command line arguments.
  
  Oh, OK. That makes sense.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7101/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7101

To: martinvonz, #hg-reviewers, mharbison72
Cc: mharbison72, mercurial-devel
phabricator - Oct. 15, 2019, 12:55 p.m.
pulkit added a comment.


  Amended the following in flight:
  
    diff --git a/tests/test-fix.t b/tests/test-fix.t
    --- a/tests/test-fix.t
    +++ b/tests/test-fix.t
    @@ -157,8 +157,10 @@ Help text for fix.
       :skipclean suboption to false.
       
       The :pattern suboption determines which files will be passed through each
    -  configured tool. See 'hg help patterns' for possible values. If there are file
    -  arguments to 'hg fix', the intersection of these patterns is used.
    +  configured tool. See 'hg help patterns' for possible values. However, all
    +  patterns are relative to the repo root, even if that text says they are
    +  relative to the current working directory. If there are file arguments to 'hg
    +  fix', the intersection of these patterns is used.
       
       There is also a configurable limit for the maximum size of file that will be
       processed by 'hg fix':

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7101/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7101

To: martinvonz, #hg-reviewers, mharbison72, pulkit
Cc: mharbison72, mercurial-devel

Patch

diff --git a/tests/test-fix.t b/tests/test-fix.t
--- a/tests/test-fix.t
+++ b/tests/test-fix.t
@@ -1321,7 +1321,7 @@ 
   $ echo modified > bar
   $ hg fix -w bar
   $ cat bar
-  modified
+  $TESTTMP/subprocesscwd
 
   $ cd ../..
 
diff --git a/hgext/fix.py b/hgext/fix.py
--- a/hgext/fix.py
+++ b/hgext/fix.py
@@ -140,6 +140,7 @@ 
     context,
     copies,
     error,
+    match as matchmod,
     mdiff,
     merge,
     obsolete,
@@ -843,7 +844,11 @@ 
 
     def affects(self, opts, fixctx, path):
         """Should this fixer run on the file at the given path and context?"""
-        return scmutil.match(fixctx, [self._pattern], opts)(path)
+        repo = fixctx.repo()
+        matcher = matchmod.match(
+            repo.root, repo.root, [self._pattern], ctx=fixctx
+        )
+        return matcher(path)
 
     def shouldoutputmetadata(self):
         """Should the stdout of this fixer start with JSON and a null byte?"""