Review Request 119792: Save the view states in addition to the view urls and splitter state in DolphinTabPage.

[Frank Reininghaus]

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


Thanks for the update - looks good!

I'm just not entirely sure if it's guaranteed that every compiler on every platform will call the same operator "<<"/">>" functions when it sees

stream << 1;

or

int version;
stream >> version;

There are overloads for 8-bit, 16-bit, 32-bit, and 64-bit integer types, and which one is used might depend on the platform. To prevent that we run into subtle bugs if saveState() uses 32-bit ints and then the restoreState() function of a different Dolphin binary expects a 64-bit int, I would suggest that the type is stated explicitly everywhere. Maybe quint32?


dolphin/src/views/dolphinview.cpp
<https://git.reviewboard.kde.org/r/119792/#comment45366>

    I think that this should be
    
    if (version != 1) {...}
    
    If the data format is changed in future versions, then this function might have trouble reading it, and a messed-up view or, even worse, a crash might be the result.
    
    One could argue that reading data created by future Dolphin versions with an older version might still work in many cases, but this won't happen very frequently anyway, so I think that the easy solution - just abort the state restoration if we see that there might be problems - makes sense here.


- Frank Reininghaus


On Aug. 20, 2014, 5 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119792/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2014, 5 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Save the view states in addition to the view urls and splitter state in DolphinTabPage.
> 
> Previously closed tabs can now be exactly restored. (same as in e.g. Firefox)
> 
> This is also great for session management, because you can continue your work exactly where you left it. :)
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphintabpage.h de5a589 
>   dolphin/src/dolphintabpage.cpp 3d1ba5a 
>   dolphin/src/dolphintabwidget.cpp 76d4b8d 
>   dolphin/src/views/dolphinview.cpp 02b8815 
> 
> Diff: https://git.reviewboard.kde.org/r/119792/diff/
> 
> 
> Testing
> -------
> 
> 1. Expand some folders, set a item somewhere in the list as current item and scroll to a random position (also in split view)
> 2. Close this tab
> 3. Reopen the previously closed tab
> 4. Check if all previously expaned folders are expanded now (same with current item and scroll position)
> 
> Works for me!
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140820/b254adde/attachment.htm>