D4364: match: document that visitchildrenset might return files

[spectral (Kyle Lippincott)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG27946fca8a05: match: document that visitchildrenset might return files (authored by spectral, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D4364?vs=10542&id=10559

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

AFFECTED FILES
  mercurial/dirstate.py
  mercurial/match.py

CHANGE DETAILS

diff --git a/mercurial/match.py b/mercurial/match.py
--- a/mercurial/match.py
+++ b/mercurial/match.py
@@ -346,7 +346,7 @@
             ----------+-------------------
              False    | set()
              'all'    | 'all'
-             True     | 'this' OR non-empty set of subdirs to visit
+             True     | 'this' OR non-empty set of subdirs -or files- to visit
 
         Example:
           Assume matchers ['path:foo/bar', 'rootfilesin:qux'], we would return
@@ -357,10 +357,21 @@
           'baz' -> set()
           'foo' -> {'bar'}
           # Ideally this would be 'all', but since the prefix nature of matchers
-          # is applied to the entire matcher, we have to downgrade to this
-          # 'this' due to the non-prefix 'rootfilesin'-kind matcher.
+          # is applied to the entire matcher, we have to downgrade this to
+          # 'this' due to the non-prefix 'rootfilesin'-kind matcher being mixed
+          # in.
           'foo/bar' -> 'this'
           'qux' -> 'this'
+
+        Important:
+          Most matchers do not know if they're representing files or
+          directories. They see ['path:dir/f'] and don't know whether 'f' is a
+          file or a directory, so visitchildrenset('dir') for most matchers will
+          return {'f'}, but if the matcher knows it's a file (like exactmatcher
+          does), it may return 'this'. Do not rely on the return being a set
+          indicating that there are no files in this dir to investigate (or
+          equivalently that if there are files to investigate in 'dir' that it
+          will always return 'this').
         '''
         return 'this'
 
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -912,11 +912,14 @@
                         continue
                     raise
                 for f, kind, st in entries:
-                    # If we needed to inspect any files, visitentries would have
-                    # been 'this' or 'all', and we would have set it to None
-                    # above. If we have visitentries populated here, we don't
-                    # care about any files in this directory, so no need to
-                    # check the type of `f`.
+                    # Some matchers may return files in the visitentries set,
+                    # instead of 'this', if the matcher explicitly mentions them
+                    # and is not an exactmatcher. This is acceptable; we do not
+                    # make any hard assumptions about file-or-directory below
+                    # based on the presence of `f` in visitentries. If
+                    # visitchildrenset returned a set, we can always skip the
+                    # entries *not* in the set it provided regardless of whether
+                    # they're actually a file or a directory.
                     if visitentries and f not in visitentries:
                         continue
                     if normalizefile:



To: spectral, #hg-reviewers
Cc: mercurial-devel