Code review comment for lp:~gz/bzr-explorer/repo_view_non_ascii_branch_open_777860

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/05/2011 06:19 PM, Martin [gz] wrote:
> Martin [gz] has proposed merging lp:~gz/bzr-explorer/repo_view_non_ascii_branch_open_777860 into lp:bzr-explorer.
>
> Requested reviews:
> Bazaar Explorer Developers (bzr-explorer-dev)
> Related bugs:
> Bug #777860 in Bazaar Explorer: "InvalidURL opening branch with non-ascii characters from repository view"
> https://bugs.launchpad.net/bzr-explorer/+bug/777860
>
> For more details, see:
> https://code.launchpad.net/~gz/bzr-explorer/repo_view_non_ascii_branch_open_777860/+merge/60090
>
> Opening branches with non-ascii names works in general, but from the repository view you get an error about invalid urls. Seems the code is a little confused about what the path is. Both osutils and urlutils are used in the file. Simply escaping the relpath (which will always be one directory as far as I can see, so we don't need to worry about slashes) in _get_abspath appears to resolve the issue at least.
>
> I've tested this manually, and would like to add an automated test. I don't see anything likely in the tests folder to base one off however.

URLs passed to transports (Transport.get()) should be URLs, and not
simple paths.

bzrlib.transport.get_transport() tries to be DWIM friendly, and will try
to figure out what kind of string is passed, and whether it should be
treated as a pure URL or as a "Unicode" URL (starts with HTTP:// but
uses u'\xb5' rather than %B5, etc.)

URLs are always 7-bit ascii.

So yes, abspath() be getting an escaped relpath. I don't know whether
all the code in that file is passing in a Unicode path string or whether
it is usually/sometimes/always a URL.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk3D/0EACgkQJdeBCYSNAAOzCACfYvolXU0FfhDNCsA80YT+g0RH
8yoAnixEEdhblUQ3THlqzlwqulrX2tTc
=Q1tp
-----END PGP SIGNATURE-----