Patchwork [RFC] match: remove doc about undefined behavior of visitdir()

login
register
mail settings
Submitter Yuya Nishihara
Date Nov. 30, 2017, 1:58 p.m.
Message ID <96068178c378074645b6.1512050314@mimosa>
Download mbox | patch
Permalink /patch/25832/
State Accepted
Headers show

Comments

Yuya Nishihara - Nov. 30, 2017, 1:58 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1512048733 -32400
#      Thu Nov 30 22:32:13 2017 +0900
# Node ID 96068178c378074645b6ecb7924ae2aa8b435be0
# Parent  4b139bb31b336c46e1b2e8f7e9f16a5332868670
match: remove doc about undefined behavior of visitdir()

This was added by 8545bd381504, but core matchers support visitdir() of
arbitrary locations since 2773540c3650, and verifier._verifymanifest()
doesn't seem to strictly obey the restriction.

I have no idea how important this API contract is for third-party extensions.
That's why this patch is RFC.
via Mercurial-devel - Nov. 30, 2017, 4:40 p.m.
Queued, thanks!

On Thu, Nov 30, 2017 at 5:58 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1512048733 -32400
> #      Thu Nov 30 22:32:13 2017 +0900
> # Node ID 96068178c378074645b6ecb7924ae2aa8b435be0
> # Parent  4b139bb31b336c46e1b2e8f7e9f16a5332868670
> match: remove doc about undefined behavior of visitdir()
>
> This was added by 8545bd381504, but core matchers support visitdir() of
> arbitrary locations since 2773540c3650, and verifier._verifymanifest()
> doesn't seem to strictly obey the restriction.

Good find! I had not realized that the in-core matchers all now handle
subdirectories correctly, but I think you are right that they do.

>
> I have no idea how important this API contract is for third-party extensions.
> That's why this patch is RFC.

I wouldn't worry about that. I think it's pretty much Google that has
cared about visitdir() and we don't have any custom matchers that
define a visitdir() that would not follow the new contract.

>
> diff --git a/mercurial/match.py b/mercurial/match.py
> --- a/mercurial/match.py
> +++ b/mercurial/match.py
> @@ -305,9 +305,6 @@ class basematcher(object):
>          Returns the string 'all' if the given directory and all subdirectories
>          should be visited. Otherwise returns True or False indicating whether
>          the given directory should be visited.
> -
> -        This function's behavior is undefined if it has returned False for
> -        one of the dir's parent directories.
>          '''
>          return True
>

Patch

diff --git a/mercurial/match.py b/mercurial/match.py
--- a/mercurial/match.py
+++ b/mercurial/match.py
@@ -305,9 +305,6 @@  class basematcher(object):
         Returns the string 'all' if the given directory and all subdirectories
         should be visited. Otherwise returns True or False indicating whether
         the given directory should be visited.
-
-        This function's behavior is undefined if it has returned False for
-        one of the dir's parent directories.
         '''
         return True