Review Request 118939: Add an option that makes it possible to open scripts and desktop files, rather than execute them

[Christoph Feck]

> On June 25, 2014, 9:48 p.m., Emmanuel Pescosta wrote:
> > Code looks good! +1 from my side
> 
> Christoph Feck wrote:
>     Should we discard this now that we have 120171?

(I really hate reviewboard... it does not really allow collaboration. You can only start a new review, and have to discard the old one, instead of simply adding in your suggested changes...)


- Christoph


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


On June 25, 2014, 4:03 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118939/
> -----------------------------------------------------------
> 
> (Updated June 25, 2014, 4:03 p.m.)
> 
> 
> Review request for Dolphin and David Faure.
> 
> 
> Bugs: 275405
>     http://bugs.kde.org/show_bug.cgi?id=275405
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> This patch adds an option to the settings dialog that makes it possible to open scripts and desktop files, rather than execute them. It is a slightly modified version of David's patch from https://bugs.kde.org/show_bug.cgi?id=275405#c12
> 
> These are the modifications:
> 
> 1. Changed the checkbox text from "Left click..." to the more general "Clicking..." because the user can revert left and right mouse buttons in the system settings.
> 
> 2. Changed the type of the option from bool to an enum. The motivation is that this makes it easier to extend this feature: when clicking an executable file, we could show a dialog that offers "Execute", "Open in {insert default application here}", maybe "Execute in an external terminal", "Cancel", and a "Do not ask again" checkbox. Note that this makes the code that deals with this setting rather verbose, unfortunately. I could not come up with a less verbose solution that still makes it clear at first sight (both in the code and the config files) what the setting is about.
> 
> I did try to implement such a dialog using convenience functions/classes like KMessageBox/QDialogButtonBox, but came to the conclusion that this is less convenient in this special case than I thought. Writing a custom dialog would probably be better. It's probably not hard, but not trivial either, and we might want to discuss with the usability team if an "Execute in Terminal" or "Execute in Terminal Panel" button makes sense.
> 
> Since we are running out of time now, I thought that getting David's patch in before the 4.14 feature and string freeze is better than nothing.
> 
> Note that "Execute" is still the default. This means that users who got used to executing scripts by clicking them will not be surprised by the change, but those who prefer to open and edit scripts have to change the setting manually. When we implement a dialog in the future, the default value should be "AlwaysAsk".
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphinapplication.cpp 8e83a85 
>   dolphin/src/dolphinmainwindow.cpp c60951d 
>   dolphin/src/dolphinviewcontainer.cpp 57452b9 
>   dolphin/src/settings/dolphin_generalsettings.kcfg 849a9c7 
>   dolphin/src/settings/general/behaviorsettingspage.h 7a9c2f0 
>   dolphin/src/settings/general/behaviorsettingspage.cpp cbbde1d 
> 
> Diff: https://git.reviewboard.kde.org/r/118939/diff/
> 
> 
> Testing
> -------
> 
> Works as expected for me, i.e., scripts are executed or opened in an editor, depending on the setting.
> 
> 
> File Attachments
> ----------------
> 
> Modified settings dialog
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/06/25/6f58e1a0-9425-42c9-9378-1327594f3733__DolphinSettingsDialog-ExecuteScripts.png
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

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