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@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
