Review Request: Allow opening files and directories by pressing 'Enter' or 'Return'

[Peter Penz]

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

Ship it!


Thanks for the update! Looks good and is exactly like the proposal you, Frank and I discussed per e-mail. As usual I've added a punch of my nitpicking stuff ;-) Please push it to master after fixing, you don't need to add another diff here.


dolphin/src/views/dolphinview.cpp
<http://git.reviewboard.kde.org/r/102450/#comment5407>

    1. const is missing: const QSet<int>...
    2. I'd suggest to use 'selection' or 'selectedItems' instead of 'itemsSet' for consistency with the other code.



dolphin/src/views/dolphinview.cpp
<http://git.reviewboard.kde.org/r/102450/#comment5408>

    Remove: It is only used in the else-path



dolphin/src/views/dolphinview.cpp
<http://git.reviewboard.kde.org/r/102450/#comment5409>

    Change to:
    if (itemsSet.isEmpty()) {
       return;
    }



dolphin/src/views/dolphinview.cpp
<http://git.reviewboard.kde.org/r/102450/#comment5410>

    Coding style - braces:
    if (itemsSet.count() == 1) {



dolphin/src/views/dolphinview.cpp
<http://git.reviewboard.kde.org/r/102450/#comment5411>

    Replace 'const int& i' by just 'int i'. The const-reference in foreach is fine for classes but unnecessary (and even more expensive) for scalar types like int.



dolphin/src/views/dolphinview.cpp
<http://git.reviewboard.kde.org/r/102450/#comment5412>

    As the KFileItem declaration has been remove above, use:
    const KFileItem fileItem = ...;



dolphin/src/views/dolphinview.cpp
<http://git.reviewboard.kde.org/r/102450/#comment5413>

    Coding style, please use:
    } else {



dolphin/src/views/dolphinview.cpp
<http://git.reviewboard.kde.org/r/102450/#comment5414>

    const KFileItem item = ...



dolphin/src/views/dolphinview.cpp
<http://git.reviewboard.kde.org/r/102450/#comment5415>

    1. const KFileItem item = ...;
    2. I'd suggest to move it down right before it is needed before 'emit requestContextMenu'


- Peter


On Aug. 29, 2011, 9:14 a.m., Tirtha Chatterjee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102450/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2011, 9:14 a.m.)
> 
> 
> Review request for KDE Base Apps and Peter Penz.
> 
> 
> Summary
> -------
> 
> Allow opening files and directories by pressing 'Enter' key. In case multiple files are selected when enter is pressed, all of them are opened. In case of multiple directories, the first directory gets opened in the current tab, while the other directories open in new tabs.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kitemlistcontroller.h 04d4985 
>   dolphin/src/kitemviews/kitemlistcontroller.cpp 207535c 
>   dolphin/src/views/dolphinview.h 437f12f 
>   dolphin/src/views/dolphinview.cpp 959e4da 
> 
> Diff: http://git.reviewboard.kde.org/r/102450/diff
> 
> 
> Testing
> -------
> 
> Yes, tested and working.
> 
> 
> Thanks,
> 
> Tirtha
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110829/a48c11d9/attachment.htm>