[Okular-devel] Review Request: Merge request for 'viewerinterface' branch

[Albert Astals Cid]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102946/#review7777
-----------------------------------------------------------



core/document.h
<http://git.reviewboard.kde.org/r/102946/#comment6656>

    This needs some documentation and a @since marker



core/document.h
<http://git.reviewboard.kde.org/r/102946/#comment6657>

    We try to maintain source and binary compatibility in the okular core library. This is source compatible but unfortunately not binary compatible, do you really need to store the viewport type? This information does not really seem to belong to the viewport to me, a viewport is a given position into the view, how you got to create it should not be part of it in my opinion. Maybe you can store something like m_lastViewportChangeBecauseOfBlaBla in document and the use it on notifyviewport?



core/global.h
<http://git.reviewboard.kde.org/r/102946/#comment6658>

    This needs @since marker



part.h
<http://git.reviewboard.kde.org/r/102946/#comment6659>

    it'd be cool if you change these bool b to bool enabled and bool show or something similar



part.cpp
<http://git.reviewboard.kde.org/r/102946/#comment6660>

    Probably 
    const QString u = QString("src:%1 %2").arg(line).arg(fileName);
    would be a bit faster?



part.cpp
<http://git.reviewboard.kde.org/r/102946/#comment6661>

    repaint is usually discouraged, does update work as well?



part.cpp
<http://git.reviewboard.kde.org/r/102946/#comment6662>

    Can you please change 
        emit(openSourceReference(absFileName, line, col));
    to 
        emit openSourceReference(absFileName, line, col);



part.cpp
<http://git.reviewboard.kde.org/r/102946/#comment6663>

    remove the outer ()



part.cpp
<http://git.reviewboard.kde.org/r/102946/#comment6664>

    You sure we do not need this anymore? Doesn't seem much related to the general patch



ui/pageview.cpp
<http://git.reviewboard.kde.org/r/102946/#comment6665>

    this is kind of misplaced since you are using d->mouseModeActionGroup a few lines above ;-)



ui/pageview.cpp
<http://git.reviewboard.kde.org/r/102946/#comment6666>

    Does update() instead of repaint() still work? update() is usually recommended
    
    Also the { should go to the next line



ui/pageview.cpp
<http://git.reviewboard.kde.org/r/102946/#comment6667>

    When will zoom have less than 0 items?


- Albert Astals Cid


On Oct. 24, 2011, 8:35 p.m., Michel Ludwig wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102946/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2011, 8:35 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> -------
> 
> The functionality that I need for Kile should now be implemented, i.e. handling of source references, disabling of certain actions and configuration options in 'ViewerWidgetMode', and drawing of source locations.
> 
> Note that currently the drawing of source locations doesn't work correctly for rotated pages. This is due to the fact that locating source references for rotated doesn't seem to be implemented in Okular yet. I'm thinking of disabling the rotate-page actions in 'ViewerWidgetMode' until this is implemented in Okular.
> 
> If you want to see the viewer mode in action, you can try it out by following the instructions given here:
> 
> http://sourceforge.net/apps/mediawiki/kile/index.php?title=Live_Preview
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt f8dcba0 
>   conf/dlggeneral.h 1ee2768 
>   conf/dlggeneral.cpp 80478e6 
>   conf/okular.kcfg b7d511c 
>   conf/preferencesdialog.h 72a7072 
>   conf/preferencesdialog.cpp 5ea6269 
>   core/document.h 2bcf280 
>   core/document.cpp a417828 
>   core/global.h 24cef77 
>   interfaces/viewerinterface.h PRE-CREATION 
>   part-viewermode.rc PRE-CREATION 
>   part.h f30eb6d 
>   part.cpp 9219db7 
>   ui/pagepainter.h 4044dd8 
>   ui/pagepainter.cpp 2bd2f23 
>   ui/pageview.h b89f346 
>   ui/pageview.cpp b119e82 
> 
> Diff: http://git.reviewboard.kde.org/r/102946/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michel Ludwig
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20111031/2c4f8226/attachment-0001.html>