| Submitter | Durham Goode |
|---|---|
| Date | Feb. 5, 2013, 11:38 p.m. |
| Message ID | <f2a1cf2cbb4ac88f922c.1360107518@dev350.prn1.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/814/ |
| State | Changes Requested, archived |
| Headers | show |
Comments
On Tue, 2013-02-05 at 15:38 -0800, Durham Goode wrote: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1360103054 28800 > # Node ID f2a1cf2cbb4ac88f922c5bcb5792c2c7c4c236d5 > # Parent c6377e34cb1ed79cb142f01652b0acfa09ef8c1f > pathauditor: add isvalidpath() method > > The pathauditor currently throws exceptions when it encounters an invalid > path. This change adds a method to allow people to treat it as a boolean. > This is currently used by scmutil.addremove and in a subsequent patch it > will be used by dirstate.walk Can we name this check()? Does it make sense to invert things so that the check is done in check() and __call__() calls check() and raises an exception on failure? > diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py > --- a/mercurial/scmutil.py > +++ b/mercurial/scmutil.py > @@ -175,6 +175,13 @@ > # want to add "foo/bar/baz" before checking if there's a "foo/.hg" > self.auditeddir.update(prefixes) > > + def isvalidpath(self, path): > + try: > + self(path) > + return True > + except (OSError, util.Abort): > + return False > + > class abstractvfs(object): > """Abstract base class; cannot be instantiated""" > > @@ -736,11 +743,7 @@ > ctx = repo[None] > walkresults = repo.dirstate.walk(m, sorted(ctx.substate), True, False) > for abs in sorted(walkresults): > - good = True > - try: > - audit_path(abs) > - except (OSError, util.Abort): > - good = False > + good = audit_path.isvalidpath(abs) > > st = walkresults[abs] > dstate = repo.dirstate[abs] > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
>> # HG changeset patch >> # User Durham Goode <durham@fb.com> >> # Date 1360103054 28800 >> # Node ID f2a1cf2cbb4ac88f922c5bcb5792c2c7c4c236d5 >> # Parent c6377e34cb1ed79cb142f01652b0acfa09ef8c1f >> pathauditor: add isvalidpath() method >> >> The pathauditor currently throws exceptions when it encounters an >>invalid >> path. This change adds a method to allow people to treat it as a >>boolean. >> This is currently used by scmutil.addremove and in a subsequent patch it >> will be used by dirstate.walk > >Can we name this check()? I'll rename it. >Does it make sense to invert things so that the check is done in check() >and __call__() calls check() and raises an exception on failure? I don't think it makes sense in this case. __call__() throws different exceptions based on how it fails, so making the algorithm return True/False and throwing a single kind of exception would give less information.
Patch
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py --- a/mercurial/scmutil.py +++ b/mercurial/scmutil.py @@ -175,6 +175,13 @@ # want to add "foo/bar/baz" before checking if there's a "foo/.hg" self.auditeddir.update(prefixes) + def isvalidpath(self, path): + try: + self(path) + return True + except (OSError, util.Abort): + return False + class abstractvfs(object): """Abstract base class; cannot be instantiated""" @@ -736,11 +743,7 @@ ctx = repo[None] walkresults = repo.dirstate.walk(m, sorted(ctx.substate), True, False) for abs in sorted(walkresults): - good = True - try: - audit_path(abs) - except (OSError, util.Abort): - good = False + good = audit_path.isvalidpath(abs) st = walkresults[abs] dstate = repo.dirstate[abs]