[PATCH] Fix Issue 2111, test-schemes unit test failure

Mads Kiilerich mads at kiilerich.com
Thu Apr 8 06:54:45 CDT 2010


Michael Glassford wrote, On 04/08/2010 01:29 PM:
> # HG changeset patch
> # User Michael Glassford<glassfordmjg at gmail.com>
> # Date 1270726094 14400
> # Node ID 443754cce2f168dad8fdc232a1b6bfe90c3380e2
> # Parent  f97b98db6fd1de6c60c1b60254c9240337dadc01
> Fix Issue 2111, test-schemes unit test failure.
> Recent Pythons (e.g. 2.6.5 and 3.1) introduce a change that causes
> urlparse.urlunparse(urlparse.urlparse('x://')) to return 'x:' instead of 'x://'.
> Fix url.hidepassword() and url.removeauth() to handle this case.
>
> diff -r f97b98db6fd1 -r 443754cce2f1 mercurial/url.py
> --- a/mercurial/url.py	Mon Apr 05 17:48:48 2010 -0500
> +++ b/mercurial/url.py	Thu Apr 08 07:28:14 2010 -0400
> @@ -11,17 +11,28 @@
>    

$ contrib/check-code.py mercurial/url.py
mercurial/url.py:17:
 >     #and urlparse.urlunparse(urlparse.urlparse('x:///y')) to return 
'x:/y' instead of 'x:///y',
  line too long
mercurial/url.py:19:
 >     #Since we know the original url, re-add the missing "//" if the 
original url had it.
  line too long
mercurial/url.py:21:
 >     if scheme and not result.startswith(scheme + '://') and 
url.startswith(scheme + '://'):
  line too long

>   from i18n import _
>   import keepalive, util
>
> +def _urlunparse(scheme, netloc, path, params, query, fragment, url):
> +    #Recent Pythons (e.g. 2.6.5 and 3.1) introduce a change that causes
> +    #urlparse.urlunparse(urlparse.urlparse('x://')) to return 'x:' instead of 'x://'
> +    #and urlparse.urlunparse(urlparse.urlparse('x:///y')) to return 'x:/y' instead of 'x:///y',
> +    #unless "x" appears in the list urlparse.uses_netloc.
> +    #Since we know the original url, re-add the missing "//" if the original url had it.
>    

We usually have space after #

> +    result = urlparse.urlunparse((scheme, netloc, path, params, query, fragment))
> +    if scheme and not result.startswith(scheme + '://') and url.startswith(scheme + '://'):
> +        result = result.replace(scheme + ':', scheme + '://')
>    

Replace seems slightly error-prone in pathological cases and there is a 
very implicit assumption about how result looks before it is fixed.

Why not add a check for
     result.startswith(scheme + ':')
and then return the result
     scheme + '://' + result[len(scheme)+1:]

> +    return result
>    

/Mads



More information about the Mercurial-devel mailing list