Merge lp:~lovesyao/bzr/windows-utf8 into lp:~bzr/bzr/trunk-old
| Status: | Work in progress |
|---|---|
| Proposed branch: | lp:~lovesyao/bzr/windows-utf8 |
| Merge into: | lp:~bzr/bzr/trunk-old |
| Diff against target: | 95 lines |
| To merge this branch: | bzr merge lp:~lovesyao/bzr/windows-utf8 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| John A Meinel | Needs Fixing on 2009-06-08 | ||
| Robert Collins (community) | 2008-11-20 | Abstain on 2009-05-06 | |
|
Review via email:
|
|||
| John A Meinel (jameinel) wrote : | # |
Something like this seems potentially very nice. However as Robert mentions, there are some changes that need to happen.
1) All of the logic should be moved out of 'bzr' and into something like 'bzrlib/
For example, if you can just subclass with:
class LoggingConsoleW
def _write(self, text):
self.
Then you can test that things are getting mapped into UTF-16, etc properly.
+ def writelines(self, text):
+ self.write(text + "\n")
+
^- I'm 95% sure that this is completely wrong. "writelines(self, lines)" is the api, where 'lines' is an iterable of strings, not a simple text string. (I'm guessing you are thinking of the Java api, writeline(), though my Java is weak, I do remember something that would add '\n' for you.)
Also, having unit tests lets us test stuff like _color() is getting called on bad characters, etc.
I think the reason you are seeing UTF-8 weirdness, is that we have defined the 'note()' functionality as writing out to a UTF-8 stream, which goes to stdout. (Which is probably also why you ended up realizing that you needed to overwrite stderr as well as stdout.)
My guess is that replacing sys.stdout and sys.stderr unilaterally is not the correct fix, and instead we should be just hooking into things like 'note()' and making sure it gets formatted correctly.
This can also get hooked into our infrastructure for "Command.
| Alexander Belchenko (bialix) wrote : | # |
> Something like this seems potentially very nice. However as Robert mentions,
> there are some changes that need to happen.
>
> 1) All of the logic should be moved out of 'bzr' and into something like
> 'bzrlib/
You mean 'bzrlib/
| Martin Pool (mbp) wrote : | # |
beyond jam's comment, it would be nice to be clear on just what bug is causing this.
I guess that it is that sys.stdout.
Supporting colors would be interesting but keying off reserved unicode characters to do it is a bit weird.
| Parth Malwankar (parthm) wrote : | # |
>
> Supporting colors would be interesting but keying off reserved unicode
> characters to do it is a bit weird.
It would be good to have bzrlib support color on windows and unix.
As this feature is not present, plugins end up with their own support. E.g. lp:bzrtools[1] and lp:bzr-grep[2] have their own implementation of color (unix only). I know there is a PyPI package (colorama[3] if I remember the code correctly) that supports color on both windows and unix. On Windows, from the developer/user perspective it translates the standard unix escape sequence into the appropriate windows equivalent. Maybe we can just use colorama or use a similar approach. Having bzrlib color support will also allow other commands to log etc. to have color.
Hmm. Maybe this should be a separate wishlist bug.
[1] http://
[2] http://
[3] http://

I'm not in a position to comment on the correctness of this. I would like to note that it really belongs deeper in the library where our existing encoding support is.