[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