Patchwork D6693: fix: ignore fixer tool configurations that are missing patterns

login
register
mail settings
Submitter phabricator
Date July 26, 2019, 4:05 p.m.
Message ID <2a019da5d04cf3f17cfd191aa6b123ef@localhost.localdomain>
Download mbox | patch
Permalink /patch/41084/
State Not Applicable
Headers show

Comments

phabricator - July 26, 2019, 4:05 p.m.
Closed by commit rHG2987d015aba4: fix: ignore fixer tool configurations that are missing patterns (authored by hooper).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".

CHANGED PRIOR TO COMMIT
  https://phab.mercurial-scm.org/D6693?vs=16062&id=16081#toc

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D6693?vs=16062&id=16081

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

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

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

CHANGE DETAILS




To: hooper, #hg-reviewers
Cc: martinvonz, 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
@@ -1242,6 +1242,26 @@ 
 
   $ cd ..
 
+Tools configured without a pattern are ignored. It would be too dangerous to
+run them on all files, because this might happen while testing a configuration
+that also deletes all of the file content. There is no reasonable subset of the
+files to use as a default. Users should be explicit about what files are
+affected by a tool. This test also confirms that we don't crash when the
+pattern config is missing, and that we only warn about it once.
+
+  $ hg init nopatternconfigured
+  $ cd nopatternconfigured
+
+  $ printf "foo" > foo
+  $ printf "bar" > bar
+  $ hg add -q
+  $ hg fix --debug --working-dir --config "fix.nopattern:command=echo fixed"
+  fixer tool has no pattern configuration: nopattern
+  $ cat foo bar
+  foobar (no-eol)
+
+  $ cd ..
+
 Test that we can configure a fixer to affect all files regardless of the cwd.
 The way we invoke matching must not prohibit this.
 
diff --git a/hgext/fix.py b/hgext/fix.py
--- a/hgext/fix.py
+++ b/hgext/fix.py
@@ -705,6 +705,14 @@ 
             setattr(fixers[name], pycompat.sysstr('_' + key),
                     attrs.get(key, default))
         fixers[name]._priority = int(fixers[name]._priority)
+        # Don't use a fixer if it has no pattern configured. It would be
+        # dangerous to let it affect all files. It would be pointless to let it
+        # affect no files. There is no reasonable subset of files to use as the
+        # default.
+        if fixers[name]._pattern is None:
+            ui.warn(
+                _('fixer tool has no pattern configuration: %s\n') % (name,))
+            del fixers[name]
     return collections.OrderedDict(
         sorted(fixers.items(), key=lambda item: item[1]._priority,
                reverse=True))
@@ -722,7 +730,8 @@ 
 
     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)
+        return (self._pattern is not None and
+                scmutil.match(fixctx, [self._pattern], opts)(path))
 
     def shouldoutputmetadata(self):
         """Should the stdout of this fixer start with JSON and a null byte?"""