vcstools / rosinstall

Other
10 stars 21 forks source link

Handle locales with non-ascii characters #77

Closed stonier closed 11 years ago

stonier commented 11 years ago

The guys had a problem with this in korean (%Z) output. This fixes it, and still works on my native english box.

Note - not guaranteed to work for all locales. I read some chinese boxes with wierdass implementations of strftime still had problems. Although, they would still have problems anyway.

tkruse commented 11 years ago

Hi Daniel, do you know more about the problem they had? E.g. did they have problems writing the file, or opening the YAML?

I'll probably rather drop the %Z entirely, but would prefer to be able to reproduce the problem to unit-test against regressions.

tkruse commented 11 years ago

The travis build fails because You missed a closing bracket, I think. But even with it, your solution breaks with python3, I believe, as we cannot invoke decode() on the result of strftime(), as in py3k this returns a unicode string that does not have a decode function. Similar (though not the same) problem here: http://www.logilab.org/ticket/82161?selected=82162&vid=ticketscreenshots those guys check the python version.

Until I find out what is wrong, can you ask someone to check whether this also fixes the problem:

-import time
+import datetime

 config_header = ("# THIS IS AN AUTOGENERATED FILE, LAST GENERATED USING %s ON %s\n"
-                     % (progname, time.strftime('%X %x %Z')))
+                     % (progname, datetime.date.today().isoformat()))
stonier commented 11 years ago

Ach, sorry. Should always do a fork/pull request rather than simple edit.

I just jumped back on to mention about python3 as well, but I see you've gotten ahead of me there.

In response to your question - their encoding is usually cp949 and it was falling over here:

https://github.com/vcstools/rosinstall/blob/master/src/rosinstall/config_yaml.py#L375

with a unicode encode decode error - it was trying to decode from the strftime under the assumption that it was ascii which failed when it hit a korean character.

I'll close this for now and follow up in the other pull request.