> On Aug. 16, 2011, 5:45 p.m., Jan Hambrecht wrote: > > Instead of passing a new bool parameter to all these methods of > > KoXmlReader, I would prefer having a single function > > setWhitespaceStrippingEnabled. This sets an internal flag which can be used > > at all the places necessary. > > Jaime Torres Amate wrote: > There are involved 1 class (KoXmlDocument) and 2 namespaces (KoXml > -public- and anonymous -private-) in the patch. > I do not like the idea of having one global variable in each of the > namespaces and in the class (or a module global variable) that are modified > by one method in the class (or in the KoXml namespace). > I do not like the idea of a single function (outside of the class) > changing the behavior of a class. > I do not like the idea of a class method changing the behavior of a > namespace. > > I would have done it if all the methods were in one class or in children > classes or in private namespaces. >
Sorry I was not clear in what I wrote. What I meant is that all the setContent methods in KoXmlDocument and the setDocument function in the KoXml namespace. So if you put a method into KoXmlDocument to set an internal flag, you can remove the stripWhitespace parameter from these setContent methods. And as setDocument gets a KoXmlDocument passed, you can check that member in that function too, using a getter from KoXmlDocument. While thinking about that, it would also be possible to pass the stripWhitespace parameter to the constructor of KoXmlDocument. Then you you only need a additional getter to check the value of the flag from within KoXml::setDocument. The other functions all belong to the implementation, so i think those can keep the additional parameter. But I think having a single place within the public interface to set that parameter results in a much cleaner interface. I hope you can agree to that. - Jan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101857/#review5747 ----------------------------------------------------------- On Aug. 15, 2011, 5:56 p.m., Jaime Torres Amate wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101857/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2011, 5:56 p.m.) > > > Review request for Calligra. > > > Summary > ------- > > Quoting the w3 consortium: > [Definition: All text that is not markup constitutes the character data of > the document.] > > And in section > http://www.w3.org/TR/REC-xml/#sec-white-space > > In editing XML documents, it is often convenient to use "white space" > (spaces, tabs, and blank lines) to set apart the markup for greater > readability. > Such white space is typically not intended for inclusion in the delivered > version of the document. > On the other hand, "significant" white space that should be preserved in the > delivered version is common, for example in poetry and source code. > > An XML processor MUST always pass all characters in a document that are not > markup through to the application. > A validating XML processor MUST also inform the application which of these > characters constitute white space appearing in element content. > > [Definition: An element type has mixed content when elements of that type may > contain character data, optionally interspersed with child elements.] > ------------------ > The attached patch modifies the xml parser to return the spaces between > and > < as text elements. > > I have added the TestXmlReaderWithoutSpaces to remove all the additional > spaces between nodes. > > The second version of this patch introduces a new method that is called only > when setContent is called with the new default boolean parameter set to false. > > > Diffs > ----- > > libs/odf/KoXmlReader.h 3f9ddf4 > libs/odf/KoXmlReader.cpp ad5e9d2 > libs/odf/tests/CMakeLists.txt ad632c8 > libs/odf/tests/TestXmlReaderWithoutSpaces.cpp PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/101857/diff > > > Testing > ------- > > The modified TestXmlReader test and the new TestXmlReaderWithoutSpaces are OK. > > Today, only the following tests failed: > > 19 - Plan-ScriptingTester (Failed) > 20 - KPlato-RCPSTester (Failed) (with a crash) > 24 - kspread-Formula (Failed) > 37 - kspread-InformationFunctions (Failed) > 41 - kspread-TextFunctions (Failed) > 43 - kspread-ValueFormatter (Failed) > 172 - libs-widgets-KoResourceTagging_test (Failed) > 193 - libs-kotext-styles-TestOpenDocumentStyle (Failed) > 196 - kotext-odf-TestChangeTracking (Failed) > > > Thanks, > > Jaime Torres > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
