Re: [Okular-devel] Review Request 125397: adding RTL reading mode feature to okular

2016-07-20 Thread Fahad Al-Saidi 


> On July 20, 2016, 6:29 p.m., Olivier Churlaud wrote:
> > Mostly style correction. Then I think it will be fine. (I'll check again)

Great. All requested changes done. Thanks


- Fahad


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


On July 21, 2016, 5:07 a.m., Fahad Al-Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125397/
> ---
> 
> (Updated July 21, 2016, 5:07 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 325650
> http://bugs.kde.org/show_bug.cgi?id=325650
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> This is my first patch to okular, so I am newbie here. This patch adds RTL 
> reading mode to okular in following view modes:
> - facing pages.
> - facing ( center first page).
> - Overview.
> 
> As well as adds “Right to left reading direction “ option to okular's 
> settings.
> 
> 
> Diffs
> -
> 
>   conf/dlggeneral.cpp 9945522 
>   conf/dlggeneralbase.ui 03d5d5d 
>   conf/okular.kcfg d90fe23 
>   ui/pageview.cpp 26373bb 
> 
> Diff: https://git.reviewboard.kde.org/r/125397/diff/
> 
> 
> Testing
> ---
> 
> I have tested in pdf & ODF files and it works fine.
> 
> 
> Thanks,
> 
> Fahad Al-Saidi
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 125397: adding RTL reading mode feature to okular

2016-07-20 Thread Fahad Al-Saidi 

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

(Updated July 21, 2016, 5:07 a.m.)


Review request for Okular and Albert Astals Cid.


Bugs: 325650
http://bugs.kde.org/show_bug.cgi?id=325650


Repository: okular


Description
---

This is my first patch to okular, so I am newbie here. This patch adds RTL 
reading mode to okular in following view modes:
- facing pages.
- facing ( center first page).
- Overview.

As well as adds “Right to left reading direction “ option to okular's settings.


Diffs (updated)
-

  conf/dlggeneral.cpp 9945522 
  conf/dlggeneralbase.ui 03d5d5d 
  conf/okular.kcfg d90fe23 
  ui/pageview.cpp 26373bb 

Diff: https://git.reviewboard.kde.org/r/125397/diff/


Testing
---

I have tested in pdf & ODF files and it works fine.


Thanks,

Fahad Al-Saidi

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 127628: [WIP] building against Qt 5

2016-07-20 Thread René J . V . Bertin 


> On July 20, 2016, 10 p.m., Olivier Churlaud wrote:
> > I think this can be dropped, as the frameworks branch is already in good 
> > shape. What do you think?

If someone has duplicated (sort of) and gone beyond my efforts then well, then 
"this" might indeed be dropped. I got side-tracked completely, I realise.
Can't think much more at the moment to be honest, I'll be away for the next few 
weeks.


- René J.V.


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


On April 10, 2016, 7:32 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127628/
> ---
> 
> (Updated April 10, 2016, 7:32 p.m.)
> 
> 
> Review request for KDE Graphics and Okular.
> 
> 
> Repository: kdegraphics-mobipocket
> 
> 
> Description
> ---
> 
> I didn't pay attention for a split second, wondered how the KF5 Okular 
> version could use a Qt4-based libqmobipocket and instead of checking (and 
> seeing the corresponding module isn't built ATM) I thought I'd try and build 
> at least libqmobipocket against Qt 5.
> 
> The result is the attached patch. Very much a work in progress, but since no 
> effort has yet been made to port this particular package to KF5 I thought I'd 
> share.
> 
> I've tried to ensure that the Qt4/KDE4 and Qt5(/KF5) versions can be 
> coinstalled under the assumption that the KDE4 header files are not installed 
> under ${prefix}/include (but e.g. under ${prefix}/include/KDE4 like on my 
> system). That can be achieved easily enough by configuring kdelibs4 with 
> `-DINCLUDE_INSTALL_DIR=${prefix}/include/KDE4`).
> 
> I'm not sure why the `STRIGI_FOUND` token isn't defined in my `TARGET_QT5` 
> cmake code path, but found a workaround which suffices for a proof-of-concept 
> implementation.
> 
> It will probably be relatively trivial to port the thumbnailer component too 
> but my not-so-rainy sunday afternoon is now over (IOW, I'll let myself be 
> beaten to it happily).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt ea2e644 
>   QMobipocket5Config.cmake PRE-CREATION 
>   QMobipocketConfig.cmake d615afc 
>   lib/CMakeLists.txt 90fc229 
>   lib/mobipocket.cpp 5b731b4 
>   lib/qmobipocket_export.h 923134a 
>   strigi/CMakeLists.txt 895284f 
> 
> Diff: https://git.reviewboard.kde.org/r/127628/diff/
> 
> 
> Testing
> ---
> 
> It builds against Qt 5.6.0 on OS X and Linux.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 125847: Change pages in presentation mode by swiping on touch screen

2016-07-20 Thread Olivier Churlaud 

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



As the port of the framework is not over, it cannot get new features. But it's 
definitely interesting, specially if okular was to be ported on Android.

- Olivier Churlaud


On Oct. 28, 2015, 9:12 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125847/
> ---
> 
> (Updated Oct. 28, 2015, 9:12 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Bugs: 354012
> http://bugs.kde.org/show_bug.cgi?id=354012
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> This patch adds support for swipe gestures to the presentation mode of 
> okular. Swiping right-to-left goes to the previous page, swiping 
> left-to-right goes to the next page.
> 
> Notes:
> 
> 1) The swipe gesture used here is a three-finger swipe, which is what the Qt 
> QSwipeGesture class implements.  I am not convinced that using three fingers 
> is optimal here, but it is the easiest to implement.  Other swiping gestures 
> are possible, but you would need to implement a custom QGestureRecognizer.  
> That would make the patch larger and more error-prone.
> 
> 2) By default, Qt5 synthesizes a mouse event for each unhandled touch event.  
> In particular, a left mouse click is synthesized for each finger tap to the 
> screen (which usually makes the presentation go to the next slide). This 
> mechanism gets in the way of gesture recognition, because the first finger 
> touch of your swipe gesture will already create a mouse-click and make your 
> presentation go to the next page, irrespective of the swipe direction.
> 
> The patch solves this problem but switching off mouse event synthesis in 
> presentation mode.  That's the line
> 
> 
> QCoreApplication::setAttribute(Qt::AA_SynthesizeMouseForUnhandledTouchEvents,false);
> 
> in the constructor of PresentationWidget.  This has short-term side effects.  
> For example, with this, you cannot finger-touch on a movie to start it 
> anymore.  The fix for this would be to implement proper handling of 
> QTouchEvents, which should not be difficult.
> 
> An alternative would be to leave mouse event synthesis turned on, but 
> implement a dummy touch event handler.  This will need only a few lines of 
> code, but it will not avoid the side effects mentioned above.
> 
> 3) This patch applies to the 'frameworks' branch.  I failed at backporting it 
> to 'master'.  There, the event loop would never receive any touchscreen 
> events at all.  This may be a Qt bug, but it may as well be my lack of 
> experience.
> 
> 
> Diffs
> -
> 
>   ui/presentationwidget.h 38bb679 
>   ui/presentationwidget.cpp 3680de1 
> 
> Diff: https://git.reviewboard.kde.org/r/125847/diff/
> 
> 
> Testing
> ---
> 
> Tested on a Thinkpad Yoga running Debian testing.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 127628: [WIP] building against Qt 5

2016-07-20 Thread Olivier Churlaud 

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



I think this can be dropped, as the frameworks branch is already in good shape. 
What do you think?

- Olivier Churlaud


On April 10, 2016, 7:32 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127628/
> ---
> 
> (Updated April 10, 2016, 7:32 p.m.)
> 
> 
> Review request for KDE Graphics and Okular.
> 
> 
> Repository: kdegraphics-mobipocket
> 
> 
> Description
> ---
> 
> I didn't pay attention for a split second, wondered how the KF5 Okular 
> version could use a Qt4-based libqmobipocket and instead of checking (and 
> seeing the corresponding module isn't built ATM) I thought I'd try and build 
> at least libqmobipocket against Qt 5.
> 
> The result is the attached patch. Very much a work in progress, but since no 
> effort has yet been made to port this particular package to KF5 I thought I'd 
> share.
> 
> I've tried to ensure that the Qt4/KDE4 and Qt5(/KF5) versions can be 
> coinstalled under the assumption that the KDE4 header files are not installed 
> under ${prefix}/include (but e.g. under ${prefix}/include/KDE4 like on my 
> system). That can be achieved easily enough by configuring kdelibs4 with 
> `-DINCLUDE_INSTALL_DIR=${prefix}/include/KDE4`).
> 
> I'm not sure why the `STRIGI_FOUND` token isn't defined in my `TARGET_QT5` 
> cmake code path, but found a workaround which suffices for a proof-of-concept 
> implementation.
> 
> It will probably be relatively trivial to port the thumbnailer component too 
> but my not-so-rainy sunday afternoon is now over (IOW, I'll let myself be 
> beaten to it happily).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt ea2e644 
>   QMobipocket5Config.cmake PRE-CREATION 
>   QMobipocketConfig.cmake d615afc 
>   lib/CMakeLists.txt 90fc229 
>   lib/mobipocket.cpp 5b731b4 
>   lib/qmobipocket_export.h 923134a 
>   strigi/CMakeLists.txt 895284f 
> 
> Diff: https://git.reviewboard.kde.org/r/127628/diff/
> 
> 
> Testing
> ---
> 
> It builds against Qt 5.6.0 on OS X and Linux.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 125397: adding RTL reading mode feature to okular

2016-07-20 Thread Olivier Churlaud 

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



Mostly style correction. Then I think it will be fine. (I'll check again)


ui/pageview.cpp (line 829)


Trailing space here to remove



ui/pageview.cpp (line 4400)


Please be sure to respect the indentation



ui/pageview.cpp (line 4416)


Still indentation and 

]else { => } else {


- Olivier Churlaud


On June 27, 2016, 2:54 p.m., Fahad Al-Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125397/
> ---
> 
> (Updated June 27, 2016, 2:54 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Bugs: 325650
> http://bugs.kde.org/show_bug.cgi?id=325650
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> This is my first patch to okular, so I am newbie here. This patch adds RTL 
> reading mode to okular in following view modes:
> - facing pages.
> - facing ( center first page).
> - Overview.
> 
> As well as adds “Right to left reading direction “ option to okular's 
> settings.
> 
> 
> Diffs
> -
> 
>   conf/dlggeneral.cpp 9945522 
>   conf/dlggeneralbase.ui 03d5d5d 
>   conf/okular.kcfg d90fe23 
>   ui/pageview.cpp 26373bb 
> 
> Diff: https://git.reviewboard.kde.org/r/125397/diff/
> 
> 
> Testing
> ---
> 
> I have tested in pdf & ODF files and it works fine.
> 
> 
> Thanks,
> 
> Fahad Al-Saidi
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 127541: Add support for importing annotations from another document.

2016-07-20 Thread Jonathan Verner 

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



I don't want to be pushy, I know that time is scarce and people (including me) 
have a lot of other things to do, but still: is there anything else that needs 
to be fixed in this patch? Is my explanation of the usecase clear and 
convincing enough?

- Jonathan Verner


On April 6, 2016, 8:01 p.m., Jonathan Verner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127541/
> ---
> 
> (Updated April 6, 2016, 8:01 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Bugs: 361292
> http://bugs.kde.org/show_bug.cgi?id=361292
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> This patch allows the user to import annotations from a different document 
> into the currently opened document.
> While importing, it tries to avoid adding duplicate annotations by looking 
> whether an annotation with 
> the same uniqueName is not already present on the given page. It doesn't 
> check whether the currently opened
> document corresponds to the one where comments are imported from. It just 
> loops over the pages starting
> from 0 to the minimum of the size of the two files and for each page it 
> imports the annotations.
> 
> 
> Diffs
> -
> 
>   core/document.h 63f58741bd6680a673945a7b7c05a10130968beb 
>   core/document.cpp 6953b1fb0dc29a375be7ff331a2a2bccce975366 
>   kdocumentviewer.h f99c69ef4ac8dbfb7ee600df7f97db7c62a409ab 
>   part.h 44d032e496aacde0b8c239ed344a0f3eb39fa09b 
>   part.cpp d8f1750f33a903c1734857042b601b52d8d8e1f2 
>   shell/shell.h fea80acfbf91968d90babd281d199341b6370bbd 
>   shell/shell.cpp d80def41c2c17364557402654205a5c705a29d1f 
>   shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 
>   tests/data/annots.pdf PRE-CREATION 
>   tests/mainshelltest.cpp a044895c1935c587552e875a98627e58d19ed443 
> 
> Diff: https://git.reviewboard.kde.org/r/127541/diff/
> 
> 
> Testing
> ---
> 
> Tested both manually on several files and also provided a simple automatic 
> test in mainshelltest.cpp.
> 
> 
> Thanks,
> 
> Jonathan Verner
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel

Re: [Okular-devel] Review Request 126445: i18n issue missing setApplicationDomain

2016-07-20 Thread Luigi Toscano 

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



This should be probably recommitted with the right author.

- Luigi Toscano


On Gen. 4, 2016, 8:12 a.m., Leslie Zhai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126445/
> ---
> 
> (Updated Gen. 4, 2016, 8:12 a.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> This patch was not implemented by myself, but by my big brother Cjacker ;-) 
> he is not familiar with git reviewboard, so I uploaded the patch for him, but 
> leave his name ;P
> 
> ```
> KLocalizedString::setApplicationDomain("okular");
> ```
> 
> 
> Diffs
> -
> 
>   shell/main.cpp 77847ba 
> 
> Diff: https://git.reviewboard.kde.org/r/126445/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Leslie Zhai
> 
>

___
Okular-devel mailing list
Okular-devel@kde.org
https://mail.kde.org/mailman/listinfo/okular-devel