D7647: Two clicks on file/folder to rename

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  In https://phabricator.kde.org/D7647#155581, @rkflx wrote:
  
  > > I likely wanted to just give focus back to Dolphin, rather than start a renaming operation.
  >
  > On the other hand, executing actions when giving focus is already happening today, e.g. activating Dolphin by clicking on a folder will enter this folder. Shouldn't we rather be consistent here?
  
  
  Right, I didn't think about that. Ok, ignore my previous comment then :)

INLINE COMMENTS

> kitemlistcontroller.cpp:587
> +
> +        if (m_selectionManager->selectedItems().count() == 1 &&
> +                    m_view->isAboveText(m_pressedIndex, m_pressedMousePos)) {

I'd keep this `if()` in a single line

> kitemlistcontroller.cpp:589
> +                    m_view->isAboveText(m_pressedIndex, m_pressedMousePos)) {
> +                    emit selectedItemTextPressed(m_pressedIndex);
> +        }

Too much indentation here.

> dolphinview.cpp:103
> +    m_versionControlObserver(0),
> +    m_twoClicksRenamingTimer(0),
> +    m_twoClicksRenamingItemIndex(-1)

let's use `nullptr` for new code

> dolphinview.cpp:649
> +        }
> +    } else if (fromTwoClicksRenamingTimer == false) {
>          RenameDialog* dialog = new RenameDialog(this, items);

`if (!fromTwoClicksRenamingTimer)`

But actually, if `fromTwoClicksRenamingTimer` would be declared inside the previous `if` block, we wouldn't need to check its value in this `else if`, right?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D7647

To: akrutzler, #dolphin, #kde_applications, elvisangelaccio, emmanuelp, ngraham, markg
Cc: rkflx, markg, funkybomber, sars, elvisangelaccio, ngraham, #konqueror
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20171015/c4f82a81/attachment.htm>