Review Request 112178: Make use of the already existing DolphinNewFileMenuObserver singleton class to achieve a better error handling (DolphinNewFileMenu)

[Emmanuel Pescosta]

> On Aug. 27, 2013, 10:29 a.m., Frank Reininghaus wrote:
> > Thanks for the new patch, but now the "File -> Create New..." menu in Dolphin is broken.
> > 
> > It doesn't matter much if the name of the action is "create_new" or "new_menu", but if you use a single action everywhere, then the action must have the same name everywhere (including all *.rc files).
> 
> Emmanuel Pescosta wrote:
>     I haven't found any usages of "create_new" in Dolphin (except of DolphinContextMenu - 1 line), so I decided to change "create_new" in DolphinContextMenu to "new_menu".
>     
>     > then the action must have the same name everywhere (including all *.rc files)
>     Where in Dolphin have you found "create_new" usages?
> 
> Emmanuel Pescosta wrote:
>     Hmm I mean DolphinNewFileMenu not DolphinContextMenu. Sry
> 
> Frank Reininghaus wrote:
>     > Where in Dolphin have you found "create_new" usages?
>     
>     In dolphinui.rc. This file defines the actions for the menu bar and the tool bar, one of which is "create_new". Similarly, the DolphinPart has the action "new_menu" in its dolphinpart.rc file.
>     
>     However, these only work if a corresponding action is created in the code. Currently, this is the case, but if you merge both actions to one, which has a single name, then you have to modify one of the .rc files as well. It doesn't matter if it's "create_new" or "new_menu", the only thing that matters is that the name of the action is the same in both .rc files and in the constructor of DolphinNewFileMenu.
>     
>     If the name in the constructor is different from the one in either .rc file, then the menu action in either Dolphin or the DolphinPart will not be there.
>     
>     Feel free to push this patch after fixing the problem.

> In dolphinui.rc. This file defines the actions for the menu bar and the tool bar, one of which is "create_new". Similarly, the DolphinPart has the action "new_menu" in its dolphinpart.rc file.
Thanks for the hint! 
I have forgotten to add *.rc to the files filter in "Find/Replace in Files" from KDevelop, so I have only found the line in DolphinNewFileMenu. Sry for the troubles! :(

> Feel free to push this patch after fixing the problem.
Thanks!


- Emmanuel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112178/#review38724
-----------------------------------------------------------


On Aug. 25, 2013, 10:13 a.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112178/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2013, 10:13 a.m.)
> 
> 
> Review request for Dolphin and David Faure.
> 
> 
> Description
> -------
> 
> Replaced all KNewFileMenu usages in DolphinPart by DolphinNewFileMenu.
> 
> Removed all signal-slot-connections related to DolphinNewFileMenu->errorMessage(QString)
> in DolphinMainWindow and DolphinContextMenu and replaced it by a better solution.
> 
> Now we make use of the already existing DolphinNewFileMenuObserver singleton class to achieve a better
> error handling, because every newly created DolphinContextMenu instance registers himself by DolphinNewFileMenuObserver
> and we use this to connect the errorMessage(QString) signal of every DolphinContextMenu instance to the errorMessage(QString)
> signal of the DolphinNewFileMenuObserver singleton class.
> 
> So we need only one connection from DolphinNewFileMenuObserver to DolphinMainWindow (or to DolphinPart) to
> collect all error messages thrown by every DolphinNewFileMenu instance.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/CMakeLists.txt 6856991 
>   dolphin/src/dolphincontextmenu.h c9840ea 
>   dolphin/src/dolphincontextmenu.cpp 3deeb38 
>   dolphin/src/dolphinmainwindow.cpp 5cc608f 
>   dolphin/src/dolphinnewfilemenu.h ae58813 
>   dolphin/src/dolphinnewfilemenu.cpp 480889f 
>   dolphin/src/dolphinpart.h 172bfaf 
>   dolphin/src/dolphinpart.cpp de4c607 
>   dolphin/src/views/dolphinnewfilemenuobserver.h 726122c 
>   dolphin/src/views/dolphinnewfilemenuobserver.cpp 1cb5739 
> 
> Diff: http://git.reviewboard.kde.org/r/112178/diff/
> 
> 
> Testing
> -------
> 
> All error messages are shown properly.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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