Review Request: Patch for dolphin that disables trash on removable media drives
Frank Reininghaus
frank78ac at googlemail.com
Fri May 11 14:30:52 BST 2012
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104915/#review13713
-----------------------------------------------------------
Thanks for the patch.
Concerning "Same behavior is already used for remote file systems, this patch just extends it for local but removable storages": I can see from the diff that the "Context Menu" part is indeed an extension of what is already done for remote files, but your change to DolphinViewActionHandler::slotTrashActivated() looks like it's doing something new for the action triggered by the "Delete" key, right?
You should follow the coding style:
- no trailing white space (marked red in reviewboard)
- a space after "if", "foreach"
- always put {...} after if/for/foreach/while/...
- opening "{" is on the same line as if/for/...
Moreover, DolphinView::isFileOnRemovableMedia() could be const, I think. Actually, I don't really see why DolphinView should be the right class to put this function, seeing that it does not access any members of DolphinView.
But please wait for Peter's feedback before working further on this.
- Frank Reininghaus
On May 11, 2012, 9:31 a.m., Alexander Skakov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104915/
> -----------------------------------------------------------
>
> (Updated May 11, 2012, 9:31 a.m.)
>
>
> Review request for Dolphin.
>
>
> Description
> -------
>
> Patch for dolphin that disables trash on removable media drives. When user opens context menu on file located on the removable media, "Move to trash" item is replaced with "Delete" item as well as "Del" keyboard shortcut executes "Delete" action instead of "Move to trash". Same behavior is already used for remote file systems, this patch just extends it for local but removable storages.
> I can modify it to be optional if there is a need for this
>
>
> Diffs
> -----
>
> dolphin/src/CMakeLists.txt c00d29c
> dolphin/src/dolphincontextmenu.cpp 286d304
> dolphin/src/views/dolphinview.h b2c4121
> dolphin/src/views/dolphinview.cpp 6a21885
> dolphin/src/views/dolphinviewactionhandler.cpp 7f23b7d
>
> Diff: http://git.reviewboard.kde.org/r/104915/diff/
>
>
> Testing
> -------
>
> On various USB flash disks, card readers, external USB HDD drives
>
>
> Thanks,
>
> Alexander Skakov
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20120511/7511e016/attachment.htm>
More information about the kfm-devel
mailing list