Re: svn commit: r1867653 - in /subversion/branches/swig-py3/subversion/bindings/swig/python/tests: trac/versioncontrol/tests/svn_fs.py utils.py

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1867653 - in /subversion/branches/swig-py3/subversion/bindings/swig/python/tests: trac/versioncontrol/tests/svn_fs.py utils.py

Branko Čibej
On 28.09.2019 08:59, [hidden email] wrote:

> Author: futatuki
> Date: Sat Sep 28 06:59:54 2019
> New Revision: 1867653
>
> URL: http://svn.apache.org/viewvc?rev=1867653&view=rev
> Log:
> On branch swig-py3: fix test for swig-py on Python 3 on Windows
>
> [ in subversion/bindings/swig/python/tests/]
>  * trac/versioncontrol/tests/svn_fs.py (REPOS_PATH, REPOS_URL),
>    On Python 3, pass a str object as argument to urllib.request.pathname2url()
>    instead of a bytes.
>  * util.py (file_uri_for_path):
>    On Python 3, pass a str object as argument to urllib.request.pathname2url()
>    instead of a bytes even if the argment `path' is a bytes object.
>
> Reported by: jcorvel

> ==============================================================================
> --- subversion/branches/swig-py3/subversion/bindings/swig/python/tests/utils.py (original)
> +++ subversion/branches/swig-py3/subversion/bindings/swig/python/tests/utils.py Sat Sep 28 06:59:54 2019
> @@ -79,7 +79,10 @@ class Temper(object):
>  
>  def file_uri_for_path(path):
>    """Return the file: URI corresponding to the given path."""
> -  uri_path = pathname2url(path).encode('UTF-8')
> +  if isinstance(path, str):
> +    uri_path = pathname2url(path).encode('UTF-8')
> +  else:
> +    uri_path = pathname2url(path.decode('UTF-8')).encode('UTF-8')

I'd write this differently:



Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1867653 - in /subversion/branches/swig-py3/subversion/bindings/swig/python/tests: trac/versioncontrol/tests/svn_fs.py utils.py

Branko Čibej
On 28.09.2019 15:39, Branko Čibej wrote:

> On 28.09.2019 08:59, [hidden email] wrote:
>> Author: futatuki
>> Date: Sat Sep 28 06:59:54 2019
>> New Revision: 1867653
>>
>> URL: http://svn.apache.org/viewvc?rev=1867653&view=rev
>> Log:
>> On branch swig-py3: fix test for swig-py on Python 3 on Windows
>>
>> [ in subversion/bindings/swig/python/tests/]
>>  * trac/versioncontrol/tests/svn_fs.py (REPOS_PATH, REPOS_URL),
>>    On Python 3, pass a str object as argument to urllib.request.pathname2url()
>>    instead of a bytes.
>>  * util.py (file_uri_for_path):
>>    On Python 3, pass a str object as argument to urllib.request.pathname2url()
>>    instead of a bytes even if the argment `path' is a bytes object.
>>
>> Reported by: jcorvel
>> ==============================================================================
>> --- subversion/branches/swig-py3/subversion/bindings/swig/python/tests/utils.py (original)
>> +++ subversion/branches/swig-py3/subversion/bindings/swig/python/tests/utils.py Sat Sep 28 06:59:54 2019
>> @@ -79,7 +79,10 @@ class Temper(object):
>>  
>>  def file_uri_for_path(path):
>>    """Return the file: URI corresponding to the given path."""
>> -  uri_path = pathname2url(path).encode('UTF-8')
>> +  if isinstance(path, str):
>> +    uri_path = pathname2url(path).encode('UTF-8')
>> +  else:
>> +    uri_path = pathname2url(path.decode('UTF-8')).encode('UTF-8')
> I'd write this differently:

First, I'd not send the message to soon (facepalm).

Then, I'd write:

    if isinstance(path, str):
      path = path.decode('UTF-8')
    uri_path = pathname2url(path).encode('UTF-8')


This way, there's only on call to pathname2url and someone who changes
the code in future doesn't have to remember to follow both branches of
the conditional.

-- Brane
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1867653 - in /subversion/branches/swig-py3/subversion/bindings/swig/python/tests: trac/versioncontrol/tests/svn_fs.py utils.py

Yasuhito FUTATSUKI
On 2019/09/28 22:42, Branko Čibej wrote:

> On 28.09.2019 15:39, Branko Čibej wrote:
>> On 28.09.2019 08:59, [hidden email] wrote:
>>> Author: futatuki
>>> Date: Sat Sep 28 06:59:54 2019
>>> New Revision: 1867653
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1867653&view=rev
>>> Log:
>>> On branch swig-py3: fix test for swig-py on Python 3 on Windows
>>>
>>> [ in subversion/bindings/swig/python/tests/]
>>>   * trac/versioncontrol/tests/svn_fs.py (REPOS_PATH, REPOS_URL),
>>>     On Python 3, pass a str object as argument to urllib.request.pathname2url()
>>>     instead of a bytes.
>>>   * util.py (file_uri_for_path):
>>>     On Python 3, pass a str object as argument to urllib.request.pathname2url()
>>>     instead of a bytes even if the argment `path' is a bytes object.
>>>
>>> Reported by: jcorvel
>>> ==============================================================================
>>> --- subversion/branches/swig-py3/subversion/bindings/swig/python/tests/utils.py (original)
>>> +++ subversion/branches/swig-py3/subversion/bindings/swig/python/tests/utils.py Sat Sep 28 06:59:54 2019
>>> @@ -79,7 +79,10 @@ class Temper(object):
>>>  
>>>   def file_uri_for_path(path):
>>>     """Return the file: URI corresponding to the given path."""
>>> -  uri_path = pathname2url(path).encode('UTF-8')
>>> +  if isinstance(path, str):
>>> +    uri_path = pathname2url(path).encode('UTF-8')
>>> +  else:
>>> +    uri_path = pathname2url(path.decode('UTF-8')).encode('UTF-8')
>> I'd write this differently:
>
> First, I'd not send the message to soon (facepalm).
>
> Then, I'd write:
>
>      if isinstance(path, str):
>        path = path.decode('UTF-8')
>      uri_path = pathname2url(path).encode('UTF-8')
>
>
> This way, there's only on call to pathname2url and someone who changes
> the code in future doesn't have to remember to follow both branches of
> the conditional.

I see. It's reasonable.

Thanks,
--
Yasuhito FUTATSUKI <[hidden email]>