Review Request: Open selected files with default application in file manager

[David Faure]

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



/trunk/KDE/kdebase/apps/dolphin/src/dolphincontroller.cpp
<http://reviewboard.kde.org/r/3163/#comment4223>

    Could be a direct member rather than a pointer, then no need for new/delete.



/trunk/KDE/kdelibs/kio/kio/kfileitemactions.cpp
<http://reviewboard.kde.org/r/3163/#comment4225>

    Make the temporary const, as in "const QStringList serviceIdList = ...".



/trunk/KDE/kdelibs/kio/kio/kfileitemactions.cpp
<http://reviewboard.kde.org/r/3163/#comment4224>

    A QString can never be "== 0", since a QString is a string, not a pointer.
    
    Use .isEmpty() instead.



/trunk/KDE/kdelibs/kio/kio/kfileitemactions.cpp
<http://reviewboard.kde.org/r/3163/#comment4229>

    Confusing indentation



/trunk/KDE/kdelibs/kio/kio/kfileitemactions.cpp
<http://reviewboard.kde.org/r/3163/#comment4226>

    KMimeTypeTrader has a preferredService method for this kind of use, no need to use query+first.



/trunk/KDE/kdelibs/kio/kio/kfileitemactions.cpp
<http://reviewboard.kde.org/r/3163/#comment4227>

    if (!serviceId.isEmpty() && ...)
    
    But even in the "empty serviceId" case, i.e. "no app associated", you need to append to "serviceItems" so that the open-with dialog appears for all these files, no? So I think the "service not empty" condition should be removed.
    



/trunk/KDE/kdelibs/kio/kio/kfileitemactions.cpp
<http://reviewboard.kde.org/r/3163/#comment4228>

    Here, on the other hand, it would be safer to test that serviceByStorageId didn't return NULL (e.g. due to a discrepancy in ksycoca).
    
    And if serviceId is empty (no app associated), you need to use KRun::displayOpenWithDialog instead of KRun::run.



/trunk/KDE/kdelibs/kio/kio/kfileitemactions.cpp
<http://reviewboard.kde.org/r/3163/#comment4232>

    This could also use KMimeTypeTrader::preferredService to make the code easier to read.



/trunk/KDE/kdelibs/kio/kio/kfileitemactions.cpp
<http://reviewboard.kde.org/r/3163/#comment4231>

    use QString() instead of 0.


- David


On 2010-03-06 21:48:09, Todd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3163/
> -----------------------------------------------------------
> 
> (Updated 2010-03-06 21:48:09)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> When files are selected in the file browser and the right-click menu is opened, adds an option above the "Open with..." sub-menu to open all the files in their respective preferred applications.  If all files have the same preferred application, it says "Open with <application>" and uses that application's icon, otherwise it just uses says "Open" and does not have an icon.
> 
> This is similar to the behavior in dolphin where selecting multiple files and hitting "enter" opens those files in the preferred application.  In order to maintain consistency, this patch also changes Dolphin to use the same slot as as the file manager right-click menu.  The old Dolphin implementation just iterated through the list of files, opening each one in its default application.  It did not have any way to deal with multiple files having the same default application.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/apps/dolphin/src/dolphincontroller.h 1080351 
>   /trunk/KDE/kdebase/apps/dolphin/src/dolphincontroller.cpp 1080351 
>   /trunk/KDE/kdelibs/kio/kio/kfileitemactions.h 1094205 
>   /trunk/KDE/kdelibs/kio/kio/kfileitemactions.cpp 1094205 
>   /trunk/KDE/kdelibs/kio/kio/kfileitemactions_p.h 1094205 
> 
> Diff: http://reviewboard.kde.org/r/3163/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Todd
> 
>