El dimarts, 14 de febrer de 2017, a les 10:26:35 CET, Jonathan Schultz va escriure: > Hear hear! > > PageView really is a a mess, I presume because it has been repeatedly > added to as okular has grown from kpdf and never been subject to a > proper clean-up. Notably (and in addition to the > PageView/PageViewPrivate confusion you mention), it has massively long > functions (eg mouseReleaseEvent has over 700 lines), overly complex and > sometimes redundant internal data structures, and its logic is quite > opaque. Not surprisingly, there are inconsistencies and bugs in the UI - > here are a couple I have raised to little or no feedback: > > https://bugs.kde.org/show_bug.cgi?id=363776 > https://bugs.kde.org/show_bug.cgi?id=361538 > https://git.reviewboard.kde.org/r/127496/ > > My personal interest stems from having forked okular for my own project > (https://github.com/jschultz/okular-tagging),
You've relicensed all of the code to GPL-3 ? Not sure that's illegal but for sure makes cherry-picking changes to the real Okular much harder. Cheers, Albert > for which I am extending > the UI. Given the apparent lack of enthusiasm for small but helpful > changes (such as your patch to use escape to leave full-screen mode, > like just about every full-screen application in the world!) I have > stopped worrying about this situation and just forged ahead with my own > code. > > Frankly I think pageview would benefit from a complete refactor/rewrite. > I see that KDE has announced a shiny new UI toolkit called Kirigami - > maybe that's the opportunity to re-write Okular's (rather clunky IMHO) UI. > > Best, > Jonathan > > On 14/02/17 07:13, habruening wrote: > > Hi Okular developers! > > > > I am trying to start with KDE development. It is my first open source > > initiative. I think, Okular is an interesting point to start. My first > > patch (https://git.reviewboard.kde.org/r/129773/) was not accepted. > > Anyway, I carry on. > > > > I think, I could try some code cleanup work. So please let me know if > > the following idea is welcome. > > > > When I see pageview.cpp, there is an object PageViewPrivate *d. > > Obviously it is PIMPL. But I cannot see a private implementation. > > Everything is done in PageView. PageViewPrivate is just a data class. I > > would like to make PageViewPrivate really a private implementation. I > > don't know yet how far I can separate PageView and PageViewPrivate. But > > I think they could have individual responsibles. One is the interface to > > the outside world and the other is all the implementation details. > > Following the single-responsibility-principle, I cannot believe that > > pageview.cpp must have 5000 lines of code. > > > > But one question: Is there a technical reason for hiding all the data in > > a d-pointer? The code looks as if the author did not want to use a > > d-pointer but was forced to it. > > > > Best regards!
