[PATCH 0 of 5] simplify new portability code

[Augie Fackler]

On May 1, 2011, at 9:22 AM, Adrian Buehlmann wrote:
> 
> On 2011-05-01 09:04, Adrian Buehlmann wrote:
>> On 2011-05-01 08:49, Adrian Buehlmann wrote:
>>> On 2011-05-01 08:22, Augie Fackler wrote:
>>>> On May 1, 2011, at 1:20 AM, Adrian Buehlmann wrote:
>>>>> 
>>>>> see patches. No functional change.
>>>>> _______________________________________________
>>>>> Mercurial-devel mailing list
>>>>> Mercurial-devel at selenic.com
>>>>> http://selenic.com/mailman/listinfo/mercurial-devel
>>>> 
>>>> Is there some particular reason to inline these functions? I feel like it doesn't add clarity to inline them in most cases, and can make it harder for extensions to reuse them in meaningful ways.
>>>> 
>>> 
>>> I feel quite the opposite.
>> 
>> And instead of the current maze of pointless inefficient misleading little
>> functions, we could do a much cleaner abstraction like this one (applies
>> *on top* of my series, untested):
>> 
> 
> Sorry, should be:
> 
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -1315,23 +1315,16 @@
>     names = []
>     wctx = repo[None]
>     wctx.status(clean=True)
> -    existing = None
> +    cca = None
>     abort, warn = scmutil.checkportabilityalert(ui)
>     if abort or warn:
> -        existing = dict([(fn.lower(), fn) for fn in
> -                         wctx.added() + wctx.clean() + wctx.modified()])
> +        cca = scmutil.casecollisionauditor(ui, abort
> +                  wctx.added() + wctx.clean() + wctx.modified())
>     for f in repo.walk(match):
>         exact = match.exact(f)
>         if exact or f not in repo.dirstate:
> -            if existing:
> -                fl = f.lower()
> -                if fl in existing and existing[fl] != f:
> -                    msg = _('possible case-folding collision for %s') % f
> -                    if abort:
> -                        raise util.Abort("%s" % msg)
> -                    elif warn:
> -                        ui.warn(_("warning: %s\n") % msg)
> -                existing[fl] = f
> +            if cca:
> +                cca(f)
>             names.append(f)
>             if ui.verbose or not exact:
>                 ui.status(_('adding %s\n') % match.rel(join(f)))
> diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
> --- a/mercurial/scmutil.py
> +++ b/mercurial/scmutil.py
> @@ -40,6 +40,22 @@
>             _("ui.portablefilenames value is invalid ('%s')") % val)
>     return abort, warn
> 
> +class casecollisionauditor(object):
> +    def __init__(self, ui, abort, existing):
> +        self._ui = ui
> +        self._abort = abort
> +        self._map = dict([(f.lower(), f) for f in existing])
> +
> +    def __call__(self, f):
> +        fl = f.lower()
> +        map = self._map
> +        if fl in map and map[fl] != f:
> +            msg = _('possible case-folding collision for %s') % f
> +            if self._abort:
> +                raise util.Abort(msg)
> +            self._ui.warn(_("warning: %s\n") % msg)
> +        map[fl] = f
> +

This looks like an improvement to me, +1 iff we include this bit.

> class path_auditor(object):
>     '''ensure that a filesystem path contains no banned components.
>     the following properties of a path are checked: