[Svnmerge] Unicode in log messages
rocketraman at fastmail.fm
Mon Oct 12 22:29:35 PDT 2009
Benson Margulies wrote:
> See patch attached to issue 3500.
Benson, first of all, thank you for the patch. Couple minor points:
please post patches to the list rather than the tracker so that it is
easier to comment on them. Also, even though svnmerge has its own
list, we follow the svn development guidelines -- please review and
I'll post some comments to your patch here this time (please reply to
these items inline to keep context rather than top posting -- it'll be
easier to discuss them individually):
1) Hunk -167,7 +167,7: don't submit unrelated items as part of the
patch. Please remove.
2) Hunk -222,11 +222,8: Why change the encoding used by the decode?
AFAIK, there is no problem with the assumption that svn is outputting
the log in the encoding returned by sys.stdout.encoding, so this
shouldn't need to change.
3) General comment: I don't really like making the commit file
encoding a user option, since I don't think the user should need to
worry about this. However, since svnmerge.py appears to be doing the
wrong thing in certain environments (like Benson's) and so far no one
seems to know what python code will consistently return the same
encoding that svn defaults to using when reading commit log messages,
I am +0 on this to provide a workaround for people in Benson's situation.
Benson, a question for you that I should have asked before: on your
machine, if you manually i.e. not via svnmerge, create a commit log
file with Arabic characters (presumably in utf-8) do you need to
explicitly specify the "--encoding utf-8" option when doing svn
commit? Or does svn use utf-8 when reading the commit log file by
default? If the former, than the current python code is actually
"correct" and the user option is definitely required. I would then be
+1 on this change -- its not unreasonable to ask the user to specify
the option to svnmerge.py if they are already doing so for svn.
4) Hunk -521,7 +518,7 and hunk -1073,7 +1070,7 and -1121,7 +1118,7:
Unnecessary, sys.stdout.encoding works for svn log output. As far as I
know this option does not affect svn log anyway -- it's for svn commit.
More information about the Svnmerge