[PATCH 1 of 8 V2] context: don't use mutable default argument value

[Gregory Szorc]

On Sun, Mar 12, 2017 at 9:57 PM, Gregory Szorc <gregory.szorc at gmail.com>
wrote:

> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1489380642 25200
> #      Sun Mar 12 21:50:42 2017 -0700
> # Node ID 265102f455de6451e493024fb8a9f24d816ce1c2
> # Parent  1c48a8278b2f015fca607dfc652823560a5ac580
> context: don't use mutable default argument value
>
> Mutable default argument values are a Python gotcha and can
> represent subtle, hard-to-find bugs. Lets rid our code base
> of them.
>

There was bikeshedding the last time I sent this series. Most of it was
over the code to enforce this with an automatic checker. Since the existing
code represents bugs, I've decided to drop that patch and just fix the code.

The previous send also involved some concerns around how to do this and
whether "foo or []" is the right pattern (versus say a dummy object you
could "is" compare against). Martijn uses the "foo or []" pattern in
d30fb3de4b40 and he knows more about Python than practically anyone. So
I'll use that to justify my opinion that "foo or []" is acceptable.


>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -298,10 +298,10 @@ class basectx(object):
>          '''
>          return subrepo.subrepo(self, path, allowwdir=True)
>
> -    def match(self, pats=[], include=None, exclude=None, default='glob',
> +    def match(self, pats=None, include=None, exclude=None, default='glob',
>                listsubrepos=False, badfn=None):
>          r = self._repo
> -        return matchmod.match(r.root, r.getcwd(), pats,
> +        return matchmod.match(r.root, r.getcwd(), pats or [],
>                                include, exclude, default,
>                                auditor=r.nofsauditor, ctx=self,
>                                listsubrepos=listsubrepos, badfn=badfn)
> @@ -1515,16 +1515,16 @@ class workingctx(committablectx):
>                      self._repo.dirstate.normallookup(dest)
>                  self._repo.dirstate.copy(source, dest)
>
> -    def match(self, pats=[], include=None, exclude=None, default='glob',
> +    def match(self, pats=None, include=None, exclude=None, default='glob',
>                listsubrepos=False, badfn=None):
>          r = self._repo
>
>          # Only a case insensitive filesystem needs magic to translate
> user input
>          # to actual case in the filesystem.
>          if not util.fscasesensitive(r.root):
> -            return matchmod.icasefsmatcher(r.root, r.getcwd(), pats,
> include,
> -                                           exclude, default, r.auditor,
> self,
> -                                           listsubrepos=listsubrepos,
> +            return matchmod.icasefsmatcher(r.root, r.getcwd(), pats or
> [],
> +                                           include, exclude, default,
> r.auditor,
> +                                           self,
> listsubrepos=listsubrepos,
>                                             badfn=badfn)
>          return matchmod.match(r.root, r.getcwd(), pats,
>                                include, exclude, default,
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170312/0e1eff8b/attachment.html>