[Svnmerge] KeyError on svnmerge.py

[Laurent PETIT]

Hello,


2008/3/8, Giovanni Bajo <rasky at develer.com>:
>
> Try restoring those two lines.


Well, if I understand well from the following messages in the thread,
restoring those two lines will reintroduce a bug if there are some uncommon
characters in the logs ?
Am I right ?

If yes, then I'll live with my patch, because, even if not perfect, it works
*right now*.
It maintains the correction for the logs, and removes the problem with
svninfo.

>
> The output of svn is meant to be machine parsable and it is very stable.
> The LC_ALL=C setting ensures that the English strings are used, which
> are not subject to i18n issues.


OK I understand, and so will throw away my patch as soon as somebody has
submitted the correction with the correct setting of environment variables,
for every environment ...

> Please find in attachment the new patch version.
>
> As I said, this patch is unnecessary, in that the same bug can be fixed
> re-adding those two lines that have been there since the inception of
> svnmerge.sh (before even svnmerge.py was born).


No, as far as I understood, just removing the two lines reintroduces a bug
concerning the correct encoding of the logs.


Moreover, your patch introduces a new dependency on xml.minidom which is
> unnecessary;


Yes, you're right. It's my first attempt with Python since 4 years, and I
didn't know well svnmerge.py codebase to detect the already existing pulldom
dependency.
Anyway, directly manipulating the dom for such a tiny output footprint for
svn info is far much efficient both in terms of coding time and lines of
code, while I guess this is just slightly less efficient in terms of
performance than pulldom in this case.

But you're right, it introduces a new dependency.
But it is a standard library of Python, isn't it ?

Anyway, I'm *not* suggesting you to write this code, since the problem
> at hand can be solved by fixing a one-liner regression. There's
> absolutely no reason to complicate the code with the xml parser unless
> there's a real-world problem we need to solve; usage of xml for svn log
> was really the only way to do it correctly.


Oh yes, I totally agree with you. I would not write an xml parser unless
striclty mandatory for good performance reason, as was obviously the case
for a quick log parsing as you did.

Finally, please consider this : the patch is certainly not perfect, but it
works well, and now, and does not reintroduce the 'log' bug (if I'm not
mislead by my english comprehension of the log problem).

Regards,

-- 
Laurent PETIT
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/svnmerge/attachments/20080308/23ab98cb/attachment.html>