Review Request: Patch for feature request 270360 - "Create new folder" in the folder panel

Frank Reininghaus frank78ac at googlemail.com
Mon Nov 12 17:42:19 GMT 2012


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


Thanks for the patch! Looks nice, except for the 'DolphinMainWindow' issue noted below, whose clean solution may require some changes in other Dolphin code.

We are in hard feature freeze and soft message freeze since Thursday [1], so I think that this has to wait until master is open again for KDE 4.11.

When master is open again, we could remove DolphinNewFileMenu's m_mainWin member and change DolphinNewFileMenu::slotResult(KJob* job) such that it emits a signal 'error(QString)' or 'errorMessage(QString)' and connect that to the main window's slot slotPanelErrorMessage(QString) (which could be renamed to 'slotErrorMessage'). I think that this is better object-oriented design and makes the parent()->parent() hack superfluous :-)

[1] http://techbase.kde.org/Schedules/KDE4/4.10_Release_Schedule


dolphin/src/panels/folders/folderspanel.cpp
<http://git.reviewboard.kde.org/r/107267/#comment16864>

    Like Peter [2], I don't quite like this line ;-)
    
    This can break easily, e.g., if we insert an additional level in the QObject hierarchy for some reason.
    
    [2] https://bugs.kde.org/show_bug.cgi?id=270360#c2


- Frank Reininghaus


On Nov. 9, 2012, 3:57 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107267/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2012, 3:57 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Patch for feature request 270360 - "Create new folder" in the folder panel
> 
> Uses the KMessageWidget to show possible error messages.
> 
> 
> This addresses bug 270360.
>     http://bugs.kde.org/show_bug.cgi?id=270360
> 
> 
> Diffs
> -----
> 
>   dolphin/src/panels/folders/folderspanel.h 14d8e87 
>   dolphin/src/panels/folders/folderspanel.cpp 6e3a767 
>   dolphin/src/panels/folders/treeviewcontextmenu.h 0b3fd79 
>   dolphin/src/panels/folders/treeviewcontextmenu.cpp fa8844d 
> 
> Diff: http://git.reviewboard.kde.org/r/107267/diff/
> 
> 
> Testing
> -------
> 
> Works fine for me (Create folders and show error messages ;)
> 
> 
> Screenshots
> -----------
> 
> New Folders Panel Context Menu
>   http://git.reviewboard.kde.org/r/107267/s/816/
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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


More information about the kfm-devel mailing list