[Okular-devel] Review Request 123427: Save view information per file

[Felix Mauch]

> On Aug. 10, 2015, 10:59 nachm., Albert Astals Cid wrote:
> > How does this code play with the other code that remembers this stuff globally? Should that code be removed?

yes, revision 2 reverts those changes, so revision 1 and 2 give the complete change. Is this the wrong way to post it? I'm sorry, I'm working with the reviewboard for the first time. Should I post complete diffs from the master instead so the different revisions will be independent?


> On Aug. 10, 2015, 10:59 nachm., Albert Astals Cid wrote:
> > ui/pageview.cpp, line 1402
> > <https://git.reviewboard.kde.org/r/123427/diff/2/?file=390795#file390795line1402>
> >
> >     You should use a string here and not an int, otherwise if in the future we add, move or remove items in this menu the numbers are going to be a pain to store/restore

I don't like hard-coded strings very much, as you'll have to change all those later on, as well. However, I agree, that my simple solution is even worse than that. What about using the data field of the action? The viewMode enum-Type from Okular::Settings is stored in there. So when new modes are added there or the menu entries are reordered, no further changes should be necessary. However, I'd still have to use an int internally, as I can't save the enum type in the QVariant. Yet, I could to a casting the

```cpp
int mode;
for (int i=0; i < d->aViewMode->menu()->actions().size(); ++i) {
    const QAction* action = d->aViewMode->menu()->actions().at(i);
    if(action->isChecked() ) {
        QVariant mode_id = action->data();
        mode = mode_id.toInt();
        break;
    }
}
return mode;
```


- Felix


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


On Aug. 6, 2015, 3:22 nachm., Felix Mauch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123427/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2015, 3:22 nachm.)
> 
> 
> Review request for Okular.
> 
> 
> Bugs: 341318
>     http://bugs.kde.org/show_bug.cgi?id=341318
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> Adds the functiionality to save the view mode (single page, facing...) and continuous scrolling to the document information as it is already done with the zoom information.
> 
> 
> Diffs
> -----
> 
>   core/document.cpp 9d12488 
>   core/view.h a369d24 
>   ui/pageview.h e65b575 
>   ui/pageview.cpp b57e6ae 
> 
> Diff: https://git.reviewboard.kde.org/r/123427/diff/
> 
> 
> Testing
> -------
> 
> I use a version with this patch applied since a couple of days. The intended functionality seems to work reliably.
> Maybe it would be nice to implement a "standard view" selection in the options dialog as it's done for the zoom mode.
> 
> 
> Thanks,
> 
> Felix Mauch
> 
>

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