refactoring in pageview class

[Albert Astals Cid]

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!