Patchwork D11715: dirstate: group return logic and clarify each function in flagfunc

login
register
mail settings
Submitter phabricator
Date Oct. 21, 2021, 7:42 a.m.
Message ID <differential-rev-PHID-DREV-qyh6ug3rtjyqtkxfr2zk-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50039/
State Superseded
Headers show

Comments

phabricator - Oct. 21, 2021, 7:42 a.m.
Alphare created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  As suggested by spectral during review of my fix of the missing return, this
  makes it more obvious that all cases are covered and improves readability of
  the functions with new names and a small docstring.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/dirstate.py

CHANGE DETAILS




To: Alphare, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -242,65 +242,68 @@ 
         return self._rootdir + f
 
     def flagfunc(self, buildfallback):
-        if self._checklink and self._checkexec:
-
-            def f(x):
-                try:
-                    st = os.lstat(self._join(x))
-                    if util.statislink(st):
-                        return b'l'
-                    if util.statisexec(st):
-                        return b'x'
-                except OSError:
-                    pass
-                return b''
+        if not (self._checklink and self._checkexec):
+            fallback = buildfallback()
 
-            return f
-
-        fallback = buildfallback()
-        if self._checklink:
-
-            def f(x):
-                if os.path.islink(self._join(x)):
+        def check_both(x):
+            """This platform supports symlinks and exec permissions"""
+            try:
+                st = os.lstat(self._join(x))
+                if util.statislink(st):
                     return b'l'
-                entry = self.get_entry(x)
-                if entry.has_fallback_exec:
-                    if entry.fallback_exec:
-                        return b'x'
-                elif b'x' in fallback(x):
+                if util.statisexec(st):
                     return b'x'
-                return b''
+            except OSError:
+                pass
+            return b''
+
+        def check_link(x):
+            """This platform only supports symlinks"""
+            if os.path.islink(self._join(x)):
+                return b'l'
+            entry = self.get_entry(x)
+            if entry.has_fallback_exec:
+                if entry.fallback_exec:
+                    return b'x'
+            elif b'x' in fallback(x):
+                return b'x'
+            return b''
 
-            return f
-        if self._checkexec:
-
-            def f(x):
-                if b'l' in fallback(x):
+        def check_exec(x):
+            """This platform only supports exec permissions"""
+            if b'l' in fallback(x):
+                return b'l'
+            entry = self.get_entry(x)
+            if entry.has_fallback_symlink:
+                if entry.fallback_symlink:
                     return b'l'
-                entry = self.get_entry(x)
-                if entry.has_fallback_symlink:
-                    if entry.fallback_symlink:
-                        return b'l'
-                if util.isexec(self._join(x)):
-                    return b'x'
-                return b''
+            if util.isexec(self._join(x)):
+                return b'x'
+            return b''
 
-            return f
-        else:
+        def check_fallback(x):
+            """This platform supports neither symlinks nor exec permissions, so
+            check the fallback in the dirstate if it exists, otherwise figure it
+            out the more expensive way from the parents."""
+            entry = self.get_entry(x)
+            if entry.has_fallback_symlink:
+                if entry.fallback_symlink:
+                    return b'l'
+            if entry.has_fallback_exec:
+                if entry.fallback_exec:
+                    return b'x'
+                elif entry.has_fallback_symlink:
+                    return b''
+            return fallback(x)
 
-            def f(x):
-                entry = self.get_entry(x)
-                if entry.has_fallback_symlink:
-                    if entry.fallback_symlink:
-                        return b'l'
-                if entry.has_fallback_exec:
-                    if entry.fallback_exec:
-                        return b'x'
-                    elif entry.has_fallback_symlink:
-                        return b''
-                return fallback(x)
-
-            return f
+        if self._checklink and self._checkexec:
+            return check_both
+        elif self._checklink:
+            return check_link
+        elif self._checkexec:
+            return check_exec
+        else:
+            return check_fallback
 
     @propertycache
     def _cwd(self):