El Dimecres, 21 de març de 2012, a les 23:26:06, Albert Astals Cid va escriure: > El Dimecres, 21 de març de 2012, a les 10:30:47, Adam Reichold va escriure: > > Hello, > > Hi > > > I attached a patch to add a simple TOC interface to the qt4 frontend as > > you proposed. This being my first try to contribute to poppler, I am very > > unsure of whether I correctly adhered to coding style and patch creation > > practices. I also had no idea of where to put the new structure > > "SimpleTocNode" I declared. (It currently just "hangs out" in > > "Poppler::Document" being declared just above the using method. This is > > surely not optimal.) So I really would appreciate helpful advice on how to > > make this fit for inclusion. > > The code is wrong in two parts. > > First, most of your new functions is shared with the old ones (they are > basically the same but instead of inserting into the QDomElement insert into > your structure). This is not acceptable since code duplication is bad. > Please reuse the existing code. > > Second, you are adding a structure with it's members public. This is a no > go. Please see how the rest of the public classes have a d pointer and > follow the same pattern.
You have three days until 0.20 feature freeze. Albert > > Cheers, > Albert > > > Concerning functionality, I tried to stick to the current ways of doing > > things as close as possible. So this really just replaces "QDomDocument" > > with a custom data structure "SimpleTocNode." I also modified the qt4 demo > > to use the simple TOC interface, but hid the modifications behind an > > #ifdef. (I think one could replace the "QString" entries with the actual > > poppler data structures like "Poppler::LinkDestination" and so on, but I > > am trying not to get ahead of myself.) > > > > Hope this this your liking. Best regards, Adam. > > > > Am 21.03.2012, 00:17 Uhr, schrieb Albert Astals Cid <[email protected]>: > > > El Dimecres, 21 de març de 2012, a les 00:03:03, Adam Reichold va > > > > > > escriure: > > >> Hello, > > >> > > >> Am 20.03.2012, 23:49 Uhr, schrieb Albert Astals Cid <[email protected]>: > > >> > El Dimarts, 20 de març de 2012, a les 14:50:59, Adam Reichold va > > >> > > > >> > escriure: > > >> >> Hello, > > >> > > > >> > Hi > > >> > > > >> >> I am currently maintaining a small PDF viewer called qpdfview that > > >> >> is > > >> >> using the poppler library through the qt4 frontend. I have three > > >> >> question > > >> >> that came up during development: > > >> >> > > >> >> - The ArthurOutputDev seems incomplete. Are there plans on this? > > >> > > > >> > There's noone working on it, patches are welcome. > > >> > > >> I feared something like that. :-) > > >> > > >> >> Can > > >> >> someone point me to documentation about this? > > >> > > > >> > There's no documentation. > > >> > > > >> >> (I would like to use it for > > >> >> printing in a platform independent way for which I currently fall > > >> > > >> back > > >> > > >> >> to > > >> >> drawing images to the printer. (I have seen a similar workaround in > > >> >> Okular > > >> >> when built on Windows.)) > > >> >> > > >> >> - The Poppler::Page::search method is deprecated but I could not > > >> >> find > > >> >> any > > >> >> documentation of what the plans for this are. Again, can someone > > >> > > >> point > > >> > > >> >> out > > >> >> relevant documentation to me? > > >> > > > >> > Use the non deprecated version of Poppler::Page::search? > > >> > > >> Oh. I assumed the version of search with the separate parameters for > > >> the > > >> rectangle was just a convenience overload. > > > > > > You asumed wrong, the deprecation marker is clearly only on one of them > > > > > > :-) > > > : > > >> Is it a long story why the first (easier?) one was deprecated? > > > > > > Because a QRect is uses qreal which are floats on some arches which > > > produces > > > comparison problems with poppler internals that use doubles. > > > > > >> >> - The Poppler::Document::toc method currently returns a QDomDocument > > >> >> pointer. Is there any other way to get the TOC using this frontend. > > >> > > > >> > No > > >> > > > >> >> I > > >> >> would like to avoid using QDomDocument as this alone would add > > >> > > >> libQtXml > > >> > > >> >> as > > >> >> a dependency to the program. Are there any opinions on replacing > > >> >> this > > >> >> by a > > >> >> simple custom tree model of the TOC? > > >> > > > >> > What's the problem of linking with libQtXml? > > >> > > >> I don't want to say that there is anything inherently wrong about it. > > >> It > > >> just feels a little excessive to link that whole library for that one > > >> feature. I feel like some of my users will care about adding > > >> dependencies. > > > > > > Sincerely I don't think your users will care about that, they care about > > > features, not if your program is written in C++, C, Java or whatnot, and > > > if > > > they care, you should be totally right to ignore them. > > > > > >> I also thought that extracting the information from the QDomDocument is > > >> a > > >> bit cumbersome considering the rather well-known structure of the TOC. > > >> But > > >> of course, I can always write some wrapper around that if really feel > > >> the > > >> need to. So this is probably just an opinion. > > > > > > If you send a patch and it's not a lot of code we might consider it for > > > inclusion. Feature for poppler 0.20 are frozen on thursday next week. > > > > > > Albert > > > > > >> > Albert > > >> > > > >> >> Thank you for your help. > > >> >> > > >> >> Best regards, Adam. > > >> >> _______________________________________________ > > >> >> poppler mailing list > > >> >> [email protected] > > >> >> http://lists.freedesktop.org/mailman/listinfo/poppler > > >> > > > >> > _______________________________________________ > > >> > poppler mailing list > > >> > [email protected] > > >> > http://lists.freedesktop.org/mailman/listinfo/poppler > > >> > > >> Thanks again. Best regards, Adam. > > >> _______________________________________________ > > >> poppler mailing list > > >> [email protected] > > >> http://lists.freedesktop.org/mailman/listinfo/poppler > > > > > > _______________________________________________ > > > poppler mailing list > > > [email protected] > > > http://lists.freedesktop.org/mailman/listinfo/poppler > > _______________________________________________ > poppler mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/poppler _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
