Re: Review Request 127541: Add support for importing annotations from another document.

2016-11-03 Thread Albert Astals Cid 


> On Nov. 3, 2016, 8:02 a.m., Jonathan Verner wrote:
> > Note: I didn't write the previous comment but my avatar still appears next 
> > to it? Should I be worried that someone has taken over my account? Or is 
> > this a bug in reviewboard? Or is this how reviewboard functions?

I'm hoping it was some someone like me with "edit all" powers, but I can't be 
sure, if noone says something, you may want to change your identity.k.o 
password just to be sure.


- Albert


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


On Nov. 3, 2016, 7:30 a.m., Jonathan Verner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127541/
> ---
> 
> (Updated Nov. 3, 2016, 7:30 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> 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
> 
>

Re: Review Request 127541: Add support for importing annotations from another document.

2016-11-03 Thread Jonathan Verner 

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



Note: I didn't write the previous comment but my avatar still appears next to 
it? Should I be worried that someone has taken over my account? Or is this a 
bug in reviewboard? Or is this how reviewboard functions?

- Jonathan Verner


On Nov. 3, 2016, 7:30 a.m., Jonathan Verner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127541/
> ---
> 
> (Updated Nov. 3, 2016, 7:30 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> 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
> 
>

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 127541: Add support for importing annotations from another document.

2016-04-06 Thread Jonathan Verner 

---
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 (updated)
-

  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 127541: Add support for importing annotations from another document.

2016-04-05 Thread Jonathan Verner 

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




shell/shell.cpp (line 428)


`m_fileformatsscanned` has to be valid at this moment and even if it were 
not, the else block would fail (This is not strictly related to the feature, 
but as I was refactoring the code anyway, I decided to get rid of this.).


- Jonathan Verner


On April 5, 2016, 7:07 a.m., Jonathan Verner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127541/
> ---
> 
> (Updated April 5, 2016, 7:07 a.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 127541: Add support for importing annotations from another document.

2016-04-05 Thread Jonathan Verner 


> On April 4, 2016, 10:26 p.m., Albert Astals Cid wrote:
> > The implementation seems ok-ish from a quick look, but is this something 
> > that really makes sense? Do you see more people other than you having a 
> > need for this this feature?

I think it does, but of course, that is my biased opinion :-) I can at least 
describe the usecases I have in mind (also included in the wishlist): if there 
are several people commenting on a draft the author would then like to merge 
their comments into a single file. Another usecase I came across was when I was 
reviewing a paper on two different computers and forgot to sync the files. I 
ended up with two sets of comments which I wanted to merge into a single file. 
I think both Acrobat and Foxit Reader have this option under windows (I found 
it in their documentation when googling for this feature but I don't have a 
windows computer handy to test it). On the other hand it sure does not need to 
feature so prominently in the file menu. Perhaps it would better fit into the 
Tools or Edit menus?


- Jonathan


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


On April 5, 2016, 7:07 a.m., Jonathan Verner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127541/
> ---
> 
> (Updated April 5, 2016, 7:07 a.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 127541: Add support for importing annotations from another document.

2016-04-05 Thread Jonathan Verner 

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

(Updated April 5, 2016, 7:07 a.m.)


Review request for Okular.


Changes
---

Fix first issue.


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 (updated)
-

  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 127541: Add support for importing annotations from another document.

2016-04-04 Thread Albert Astals Cid 

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



The implementation seems ok-ish from a quick look, but is this something that 
really makes sense? Do you see more people other than you having a need for 
this this feature?


shell/shell.cpp (line 432)


You can't do i18n(caption), the relevant text won't be extracted since it 
won't be marked with i18n, just move the i18n to the proper places.


- Albert Astals Cid


On April 1, 2016, 9:12 p.m., Jonathan Verner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127541/
> ---
> 
> (Updated April 1, 2016, 9:12 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

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

2016-04-01 Thread Jonathan Verner 

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

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