Review Request 118805: DolphinRecentTabsMenu

[Frank Reininghaus]

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

Ship it!


Looks good and works fine for me, thanks!

Just two little things that might be worth looking at either now or after this commit is in:

1. The comment that says that up to 8 closed tabs are shown in the menu is wrong. It's only 6 because the "Clear" action and the separator are included in the count.

2. I think that removeAction(QAction*) does not actually delete the action. Therefore, we might have a memory leak here. Valgrind would not report it because the actions are still children of the menu and thus deleted when the process ends, but if the user opens and closes lots of tabs, the memory usage might grow slowly, but indefinitely. So I think that we should add delete statements, unless my assumption that removeAction does not delete is wrong.

- Frank Reininghaus


On June 18, 2014, 10:28 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118805/
> -----------------------------------------------------------
> 
> (Updated June 18, 2014, 10:28 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Implemented DolphinRecentTabsMenu to encapsulate the recent
> tabs menu related code from DolphinMainWindow in a new class.
> 
> The DolphinRecentTabsMenu remembers the tab configuration if a
> tab has been closed.
> 
> Branch-Commit: 971391b270a03d9ddb90fddfbabec5cd0a8f1b5c
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphinrecenttabsmenu.cpp PRE-CREATION 
>   dolphin/src/dolphinrecenttabsmenu.h PRE-CREATION 
>   dolphin/src/dolphinmainwindow.cpp 0ad224c 
>   dolphin/src/dolphinmainwindow.h cb97612 
>   dolphin/src/CMakeLists.txt 0a72721 
> 
> Diff: https://git.reviewboard.kde.org/r/118805/diff/
> 
> 
> Testing
> -------
> 
> Closing a tab -> appears in the recent tabs menu
> 
> Choosing a previously closed tab from recent tabs
> menu an click on it -> closed tab will be opened
> again and is also removed from recent tabs menu
> 
> Closing more than 8 tabs -> oldest closed tabs
> will be removed (FIFO - maximum 8 entries)
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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