[PATCH 1 of 2 v3] scmutil: refactor ui.portablefilenames processing

Adrian Buehlmann adrian at cadifra.com
Sat Apr 30 11:05:41 CDT 2011


On 2011-04-30 14:37, Kevin Gessner wrote:
> # HG changeset patch
> # User Kevin Gessner <kevin at kevingessner.com>
> # Date 1304154504 -7200
> # Node ID 7d8f45c157752f91ae4daa590146463c1bbb304c
> # Parent  3c616f512a5b55dfc269b7da5818df66b286c680
> scmutil: refactor ui.portablefilenames processing
> 
> The ui.portablefilenames config handling is generally useful for notifying the
> user of various portability problems.
> 
> diff -r 3c616f512a5b -r 7d8f45c15775 mercurial/scmutil.py
> --- a/mercurial/scmutil.py	Fri Apr 29 22:21:13 2011 +0300
> +++ b/mercurial/scmutil.py	Sat Apr 30 11:08:24 2011 +0200
> @@ -17,19 +17,37 @@
>  def checkportable(ui, f):
>      '''Check if filename f is portable and warn or abort depending on config'''
>      checkfilename(f)
> +    if showportabilityalert(ui):
> +        msg = util.checkwinfilename(f)
> +        if msg:
> +            portabilityalert(ui, "%s: %r" % (msg, f))

This calls checkportabilityalert twice? Works, but not particularly
elegant...

> +def checkportabilityalert(ui):
> +    '''check if the user's config requests nothing, a warning, or abort for
> +    non-portable filenames'''
>      val = ui.config('ui', 'portablefilenames', 'warn')
>      lval = val.lower()
> +    bval = util.parsebool(val)
>      abort = os.name == 'nt' or lval == 'abort'
> -    bval = util.parsebool(val)
> -    if abort or lval == 'warn' or bval:
> -        msg = util.checkwinfilename(f)
> -        if msg:
> -            if abort:
> -                raise util.Abort("%s: %r" % (msg, f))
> -            ui.warn(_("warning: %s: %r\n") % (msg, f))
> -    elif bval is None and lval != 'ignore':
> +    warn = bval or lval == 'warn'
> +    if bval is None and not (warn or abort or lval == 'ignore'):
>          raise error.ConfigError(
>              _("ui.portablefilenames value is invalid ('%s')") % val)
> +    return abort, warn
> +

..

> +def showportabilityalert(ui):
> +    '''check if the user wants any notification of portability problems'''
> +    abort, warn = checkportabilityalert(ui)
> +    return abort or warn

Seems a bit overkill to me...

> +def portabilityalert(ui, msg):
> +    if not msg:
> +        return

Is any caller calling this function with msg == None or ''?

> +    abort, warn = checkportabilityalert(ui)
> +    if abort:
> +        raise util.Abort("%s" % msg)
> +    elif warn:
> +        ui.warn(_("warning: %s\n") % msg)
>  
>  class path_auditor(object):
>      '''ensure that a filesystem path contains no banned components.


More information about the Mercurial-devel mailing list