GNOME Bugzilla – Bug 160262
intltool doesn't inherit xml:space attribute
Last modified: 2005-06-26 10:57:52 UTC
As specified by the XML recommendation, the xml:space attribute needs to be inherited by children, if a parent tag specifies it. We need to do this in intltool as well. Or perhaps we just need to preserve whitespace by default. I believe we used to. If I knew how to inherit the property with XML::Parser, I'd fix it myself. But, I don't... so, Danilo, can you please fix this? We need to do this for Evolution. Thanks.
Uhm, it seems we need to handle it ourselves, since XML::Parser style doesn't help there automatically. I'll work on a patch.
Another thing: intltool-extract is not yet using XML::Parser (I planned to switch to it, and that's why I have rewritten XML parsing in intltool-merge to separate out getXMLString which will be useful in intltool-extract as well), so we cannot extract such messages correctly. Now, in what ways are you using this in Evolution, so I can see if we can at least get a simpler quick fix? (If you don't need these to be extracted for translation, but just intltool-merge to respect them, it's much easier at this time :)
Created attachment 34447 [details] [review] Fix intltool-merge to correctly handle xml:space inheritance This fixes intltool-merge at least. Fixing intltool-extract will involve switching over to XML::Parser as well. 2004-12-03 Danilo Šegan <dsegan@gmx.net> Part of fix for #160262: Correct handling of xml:space inheritance. * intltool-merge.in.in (traverse): Add $spacepreserve parameter. (translate_subnodes): Add $spacepreservee parameter. (getXMLstring): Add $spacepreserve parameter.
Evolution isn't using intltool yet for the error xml files. I'm working on moving it to do so, but this bug was a blocker for that. I think the extraction might be fine right now, if intltool-extract preserves whitespace, but I think we need to fix it to use XML::Parser too, if we are going to extract strings from XML for translation, but it is probably a lower priority.
intltool-extract preserves whitespace only when xml:space="preserve" is present on tag it is extracting (and always on attributes). Spaces are stripped for a reason (which is that translators like that), and we won't go back to non-stripping since it will cause major compatibility problems. This means that you can't do the following: <root> <tag xml:space="preserve"> <_translate> Spaces are preserved </_translate> </tag> </root> Of course, switch to XML::Parser is planned already (I have even started working on it a while ago), but since I want to avoid any regressions, I'm taking my time (i.e. I don't want to lose ability to extract comments for translators, and I want to make as much of code reusable between both intltool-merge and intltool-extract, so we avoid many problems in front).
Shouldn't the block of XML you just specified, work, since it has xml:space="preserve"?
Yes, it *should*, but intltool-extract uses regular expressions to extract XML, so you can guess why it doesn't work correctly (that's why I pointed this example out). intltool-extract would extract message as "Spaces are preserved" instead of "\nSpaces\nare\npreserved\n".
Hrmm. Your patch doesn't seem to fix the problem. xml:space="preserve" is set on the root node in my xml.in, but teh resulting .xml still has the whitespace removed. The example you pasted here should be a suitable test, but move the xml:space attribute to the root node instead. Thanks dude.
Created attachment 34473 [details] [review] Fix intltool-merge to correctly handle xml:space inheritance Updated patch. 2004-12-04 Danilo Šegan <dsegan@gmx.net> Part of fix for #160262: Correct handling of xml:space inheritance. * intltool-merge.in.in (traverse): Add $spacepreserve parameter. (translate_subnodes): Add $spacepreservee parameter. (getXMLstring): Add $spacepreserve parameter. (parseTree): Set $spacepreserve for root node as well.
Uhm, now that I think of this, I also need to check for xml:space="default" and reset $spacepreserve to 0 in nested tags when I come across that.
Yeah, you do. I think it is a lower priority though and won't happen as often.
Hrmm. Your latest patch still isn't working for me either. :(
Ugh. Ignore me. I am stupid. I copied a broken version of the xml over to xml.in to do the intltool conversion, without realizing it. So of course it didn't work :P.
Such confusion. Even after fixing the xml.in file to be correct, it still isn't working for me. Here's a block to test with: <?xml version="1.0" encoding="UTF-8"?> <error-list domain="shell" xml:space="preserve"> <error id="upgrade-nospace" type="error"> <_primary>Insufficient disk space for upgrade.</_primary> <_secondary>Upgrading your data and settings will require up to {0} of disk space, but you only have {1} available. You will need to make more space available in your home directory before you can continue.</_secondary> <button stock="gtk-quit" response="GTK_RESPONSE_CANCEL"/> </error> </error_list>
Danilo? Can you look at this? Your patches aren't working for me, and we really need this to work properly for Evolution. Thanks.
Rodney, this is what I explained above: you want intltool-extract patches, and that's not very trivial. My patch above would only work for cases where there're no messages to extract (intltool-merge would output expected XML in non "_"-marked tags). As I also said, intltool-extract is not yet using XML::Parser itself, so I'll up my priority on doing that, and I'll try to get there before January 10th (feature freeze for Gnome 2.10), but I make no promises.
Created attachment 35739 [details] [review] Switch intltool-extract to XML::Parser as well This patch switches intltool-extract to XML::Parser as well, and I fix xml:space handling along the way. I tried to do as much verbatim copying from intltool-merge.in.in as I could (that was also my motivation in restructuring XML handling code in intltool-merge), but it would need more cleaning-up. I DON'T want to commit this before it's all cleaned up, and before I add comment support as well (otherwise, we would have a regression in intltool-extract, and that won't happen). Of course, I want to do more testing as well. I suggest you use intltool with this patch internally in Evolution until I clean it up (it would involve cleaning up code in intltool-merge as well, and I noticed quite a few areas that would benefit from being looked over). This patch is something I'd call "minimal changes from intltool-merge", it's far from optimal! [Note that I should handle xml:space="default" now as well, though I didn't test it; I tested your above example, and it seems to work fine, as do all the testcases, except the comment extraction] 2005-01-09 Danilo 各gan <dsegan@gmx.net> Fix for #160262: Correct handling of xml:space inheritance. Switch intltool-extract to XML::Parser as well. * intltool-extract.in.in (intltool_tree_cdatastart) (intltool_tree_cdataend, intltool_tree_char) (intltool_tree_start): Copied verbatim from intltool-merge.in.in. (parseTree): Verbatim copy from intltool-merge.in.in. (getXMLstring): Verbatim copy from intltool-merge.in.in. (translate_subnodes): Copied from intltool-merge.in.in and removed MULTIPLE_OUTPUT handling. (readXml): Added, parse string as XML. (traverse): Added. (getAttributeString): Added. (type_xml): Use XML::Parser for extracting XML messages. * intltool-merge.in.in (traverse): Add $spacepreserve parameter. (translate_subnodes): Add $spacepreservee parameter. (getXMLstring): Add $spacepreserve parameter. (parseTree): Set $spacepreserve for root node as well.
Danilo, can you commit the intltool-merge.in.in portion of your patching for this? And also a test case if possible? Thanks.
Created attachment 48345 [details] [review] Switch intltool-extract to XML::Parser, minor cleanups 2005-06-26 Danilo Šegan <dsegan@gmx.net> Fix #160262: Correct handling of xml:space inheritance. * tests/selftest.pl.in: Added testcases 25 and 26 for inherited xml:space. * tests/cases/space-preserve.xml.in: * tests/cases/spacepreserve.po: * tests/results/space-preserve.xml.in.h: * tests/results/space-preserve.xml: Added testcase. * tests/results/merge-deepattr.xml: Updated testcase. * tests/results/extract-comments.xml.h: * tests/cases/extract-comments.xml: Updated testcase. * intltool-extract.in.in (intltool_tree_comment) (intltool_tree_cdatastart, intltool_tree_cdataend) (intltool_tree_char, intltool_tree_start, getXMLstring): Verbatim copy from intltool-merge.in.in. (readXml): Copied from intltool-merge.in.in and added comment handler. (getAttributeString): New function. * intltool-merge.in.in (intltool_tree_comment): Added for compatibility with intltool-extract. (getAttributeString, getXMLstring): Some code clean-ups. (traverse): Add $spacepreserve parameter. (translate_subnodes): Add $spacepreserve parameter. (getXMLstring): Add $spacepreserve parameter. (parseTree): Set $spacepreserve for root node as well.
I've committed this (along with a test case which checks both inheriting xml:space="preserve" and inheriting/reverting to xml:space="default"). Please test as much as possible, since we're finally using XML::Parser in intltool-extract as well (which fixes some other bugs, such as not being able to use ">" in a comment for translators).