[webkit-dev] Re: [webkit-changes]  trunk/WebCore
hyatt at apple.com
Fri Jul 27 15:59:26 PDT 2007
Didn't KDOM have an XML parsing abstraction? Would that be worth
On Jul 27, 2007, at 3:26 PM, Maciej Stachowiak wrote:
> On Jul 27, 2007, at 11:36 AM, Lars Knoll wrote:
>> On Friday 27 July 2007 16:50:01 Darin Adler wrote:
>>> I'm not happy with the code organization here. XMLTokenizer now has
>>> tons of ifdefs and two separate implementations. It's fine to have a
>>> QXmlStream implementation, but the two implementations should be in
>>> separate files, side by side, as we do in the platform layer.
>> I agree that the #ifdef's are not optimal, and we can move parts
>> of the code
>> out. But currently a lot of code is still shared between the
>> QXmlStream and
>> the libxml based implementations, so we have two choices:
>> 1. just add an XMLTokenizerQt.cpp and duplicate lots of code.
>> 2. add a qt/XMLTokenizerQt.cpp and a libxml/XMLTokenizerLibXml.cpp
>> and keep a
>> common XMLTokenizer.cpp for code that is used in both.
>> 3. Have some ifdef's in the one file.
>> The first option is no good, as we'd have to duplicate lot's of
>> shared code
>> leading to maintenance issues in the long term.
>> The second option is better, but also here we'd have quite a bit more
>> duplicated code than required as long as we don't do some bigger
>> to abstract out the common parts. This is something we wanted to
>> avoid at the
>> moment. So we went for option 3.
> Other organizations have requested the ability to use other XML
> parsers as well, such as expat. Seems like in the long run we want
> a different approach than just ifdefs in the XMLTokenizer.cpp file.
> It seems like the best would be some abstraction layer on top of
> the parser library, but if that is difficult then your option #2
> sounds like a docent long-run approach. I would have expected just
> about every XML parsing library to have a SAX-like API, which
> shouldn't be too hard to abstract, but perhaps QXml works differently.
>>> Or the XML parser should be abstracted out so we have a single
>>> XMLTokenizer built on a single parser abstraction.
>> That will be IMO almost impossible to do in an efficient way as
>> the two
>> parsers work very differently.
>>> I think this is also the type of patch that should be reviewed by at
>>> least one person other than the folks concentrating on the Qt port.
>> We've had other people look at most patches. The QXmlStream
>> changes are
>> required, and I think the only issue to discuss here could be
>> implementations. This is something I'd like to do in the longer
>> term, but as
>> you're trying to stabilize currently I didn't feel that this
>> should have been
>> done right now.
>>> With the huge number of check-ins today, I'm sure there are other
>>> interesting things like this that I missed.
>> I think we did a thorough job reviewing our patches and testing
>> them on all
>> platforms (Mac, Windows, gtk and Qt). We also kept most changes
>> rather small
>> and atomic to make reviewing easier.
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
More information about the webkit-dev