Review Request 120171: Add option to allow execution as well as opening of scripts and desktop files

[David Faure]

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


The general direction looks ok to me (but the dolphin maintainer will have the final word on that). Some comments.


dolphin/src/dolphinmainwindow.cpp
<https://git.reviewboard.kde.org/r/120171/#comment46328>

    missing space after if



dolphin/src/dolphinmainwindow.cpp
<https://git.reviewboard.kde.org/r/120171/#comment46330>

    mime->name() is simpler to read



dolphin/src/dolphinmainwindow.cpp
<https://git.reviewboard.kde.org/r/120171/#comment46329>

    should be mime->is("application/x-desktop") to take inheritance into account (just in case)



dolphin/src/dolphinmainwindow.cpp
<https://git.reviewboard.kde.org/r/120171/#comment46331>

    strange indentation, no? (I don't know the dolphin coding style though)



dolphin/src/dolphinmainwindow.cpp
<https://git.reviewboard.kde.org/r/120171/#comment46332>

    the nested switch makes the method a bit hard to read, maybe split the whole handling of "always ask" into a separate method?



dolphin/src/dolphinmainwindow.cpp
<https://git.reviewboard.kde.org/r/120171/#comment46333>

    I guess you mean url.toLocalFile()
    
    (path and toLocalFile do the same on Unix for local paths but not on Windows, e.g. file:///c:/a.txt leads to path=/c:/a.txt (directly extracted from the path component of the URL) and toLocalFile=c:/a.txt (which QFile can use))



dolphin/src/panels/terminal/terminalpanel.cpp
<https://git.reviewboard.kde.org/r/120171/#comment46334>

    toLocalFile()
    
    (in all cases this assumes that the url is local, so either assert that here or verify callers)


- David Faure


On Sept. 12, 2014, 8:54 p.m., Arjun Ak wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120171/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2014, 8:54 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 allows the user to execute (in background, within the embedded terminal, external terminal) or open an executable file (scripts, desktop files...). When the user clicks on an executable file, a [dialog](https://git.reviewboard.kde.org/media/uploaded/files/2014/09/12/0fa53887-666f-4121-9ec6-9f323d633d03__dialog.png) pops up which contains options to either open it or execute it. Clicking on execute runs the program in the background (dolphin's current behaviour).
> 
> The [settings dialog](https://git.reviewboard.kde.org/media/uploaded/files/2014/09/12/cd4d14d3-dd82-4c2c-8c0a-c2a428c78903__settings.png) has further options to fine-tune how the file is executed. The program can either be executed in the background or inside the embedded konsole or in an external terminal.
> 
> In case there is a process already running in the embedded terminal, with [user's permission](https://git.reviewboard.kde.org/media/uploaded/files/2014/09/12/c5d7843a-099a-43d4-a2e1-6ce429623866__replace.png) it will be terminated and the selected program will be started.
> 
> 
> See also:
> https://git.reviewboard.kde.org/r/118939/
> 
> 
> Diffs
> -----
> 
>   dolphin/src/settings/dolphin_generalsettings.kcfg 849a9c7 
>   dolphin/src/settings/general/behaviorsettingspage.h 7a9c2f0 
>   dolphin/src/settings/general/behaviorsettingspage.cpp cbbde1d 
>   dolphin/src/views/executablefileopendialog.h PRE-CREATION 
>   dolphin/src/views/executablefileopendialog.cpp PRE-CREATION 
>   dolphin/src/CMakeLists.txt 6f256a2 
>   dolphin/src/dolphinmainwindow.h 9d4c003 
>   dolphin/src/dolphinmainwindow.cpp 95b08af 
>   dolphin/src/dolphinviewcontainer.h 31612f1 
>   dolphin/src/dolphinviewcontainer.cpp a11ba42 
>   dolphin/src/panels/terminal/terminalpanel.h 374476e 
>   dolphin/src/panels/terminal/terminalpanel.cpp bfd3002 
> 
> Diff: https://git.reviewboard.kde.org/r/120171/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> Dialog
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/09/12/0fa53887-666f-4121-9ec6-9f323d633d03__dialog.png
> settings
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/09/12/cd4d14d3-dd82-4c2c-8c0a-c2a428c78903__settings.png
> Replace current program
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/09/12/c5d7843a-099a-43d4-a2e1-6ce429623866__replace.png
> 
> 
> Thanks,
> 
> Arjun Ak
> 
>

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