[webkit-dev] Re: [webkit-changes] [24723] trunk/WebCore

[David Hyatt]

Didn't KDOM have an XML parsing abstraction?  Would that be worth  
studying?

dave

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  
>> refactoring
>> 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.
>
> Regards,
> Maciej
>
>
>>
>>
>>> 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  
>> separating
>> 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.
>>
>> Lars
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev