[Okular-devel] Review Request 127013: Properly show marker for current section in TOC

Albert Astals Cid aacid at kde.org
Wed Feb 17 22:43:07 UTC 2016



> On Feb. 11, 2016, 10:27 p.m., Miklós Máté wrote:
> > ui/toc.cpp, line 44
> > <https://git.reviewboard.kde.org/r/127013/diff/1/?file=443811#file443811line44>
> >
> >     The model has to know about the treeview, see line 209. This also makes sure that the model is deleted when the view is deleted, just like when this is used as a KPart (see finishReload).
> 
> Albert Astals Cid wrote:
>     > This also makes sure that the model is deleted when the view is deleted
>     
>     But that already happens becase the view is the parent of the model, no?
> 
> Miklós Máté wrote:
>     You have to call setParent() to make the view parent of the model, it doesn't happen automatically. You can connect multiple views to a single model, but only one of them can be its parent, or they would all want to free the model on exit.
> 
> Albert Astals Cid wrote:
>     Really, read the code, specially the tocmodel constructor and see that we call m_model = new TOCModel( document, m_treeView );
>     
>     Do you really not see that the model parent is already the treeview? 
>     
>     Just add qDebug() << "MOOOOOOOOO" << m_model->QObject::parent() << m_treeView; after the constructor and see it yourself
> 
> Miklós Máté wrote:
>     OK, you win. I didn't notice that constructor. The change to toc.cpp is unnecessary.

This is not about winning, it's about producing good code. This is not me vs you, it's me with you trying to get the code in the best possible shape to land it.


- Albert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127013/#review92277
-----------------------------------------------------------


On Feb. 8, 2016, 5:49 p.m., Miklós Máté wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127013/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2016, 5:49 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> fixes bug #342076
> 
> 
> Diffs
> -----
> 
>   ui/toc.cpp 09625af 
>   ui/tocmodel.cpp ce93366 
> 
> Diff: https://git.reviewboard.kde.org/r/127013/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Miklós Máté
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20160217/fe5347ba/attachment.html>


More information about the Okular-devel mailing list