[merge] win32 cleanups
Alexander Belchenko
bialix at ukr.net
Sun Jun 11 06:11:07 BST 2006
John Arbash Meinel пишет:
> Anyway, I wanted to let people know these fixes were out there, and
> maybe get a review/+1. Even though not all tests are passing, more tests
> pass now, and it shouldn't break Linux stuff.
I try to review your patch.
> ------------------------------------------------------------------------
>
> === modified file 'bzrlib/commands.py'
> --- bzrlib/commands.py 2006-06-07 17:23:59 +0000
> +++ bzrlib/commands.py 2006-06-11 03:22:37 +0000
> @@ -41,6 +41,7 @@
> BzrOptionError,
> NotBranchError)
> from bzrlib.option import Option
> +import bzrlib.osutils
> from bzrlib.revisionspec import RevisionSpec
> from bzrlib.symbol_versioning import *
> import bzrlib.trace
> @@ -235,16 +236,15 @@
> self.outf = sys.stdout
> return
>
> - output_encoding = getattr(sys.stdout, 'encoding', None)
> - if not output_encoding:
> - output_encoding = bzrlib.user_encoding
> - mutter('encoding stdout bzrlib.user_encoding %r', output_encoding)
> - else:
> - mutter('encoding stdout log as sys.stdout encoding %r', output_encoding)
> + output_encoding = bzrlib.osutils.get_terminal_encoding()
>
> # use 'replace' so that we don't abort if trying to write out
> # in e.g. the default C locale.
> self.outf = codecs.getwriter(output_encoding)(sys.stdout, errors=self.encoding_type)
> + # For whatever reason codecs.getwriter() does not advertise its encoding
> + # it just returns the encoding of the wrapped file, which is completely
> + # bogus. So set the attribute, so we can find the correct encoding later.
> + self.outf.encoding = output_encoding
>
> @deprecated_method(zero_eight)
> def run_argv(self, argv):
>
OK.
> === modified file 'bzrlib/msgeditor.py'
> --- bzrlib/msgeditor.py 2006-06-06 12:06:20 +0000
> +++ bzrlib/msgeditor.py 2006-06-11 03:22:39 +0000
> @@ -26,6 +26,7 @@
> import bzrlib
> import bzrlib.config as config
> from bzrlib.errors import BzrError
> +from bzrlib.trace import mutter
>
>
> def _get_editor():
> @@ -56,6 +57,7 @@
> for e in _get_editor():
> edargs = e.split(' ')
> try:
> + ## mutter("trying editor: %r", (edargs +[filename]))
> x = call(edargs + [filename])
> except OSError, e:
> # We're searching for an editor, so catch safe errors and continue
>
Probably this part you were used to track down problem with spawning
'del' command, so actually it don't needed anymore. You can remove this
stuff.
[Then skip to cite big part of patch because it seems correct]
@@ -320,11 +327,12 @@
>
> if sys.platform == 'win32' and url.startswith('file:///'):
> # Strip off the drive letter
> - if path[2:3] not in '\\/':
> + # path is currently /C:/foo
> + if path[2:3] not in ':|' or path[3:4] not in '\\/':
> raise errors.InvalidURL(url,
> 'win32 file:/// paths need a drive letter')
> - url_base += path[1:4] # file:///C|/
> - path = path[3:]
> + url_base += path[0:3] # file:// + /c:
> + path = path[3:] # /foo
>
You are using path[2:3] in situation when path[2] is enough. Your
variant seems to me slightly unclear, because you actually need only one
character but not slice. I can understand your point, it's just my -0.02.
--
Alexander
More information about the bazaar
mailing list