D24505: Avoid emitting twice twice selectionChanged when keyboard changes the selection

Elvis Angelaccio noreply at phabricator.kde.org
Sun Oct 20 10:50:23 BST 2019


elvisangelaccio added a comment.


  In D24505#546485 <https://phabricator.kde.org/D24505#546485>, @elvisangelaccio wrote:
  
  > I still don't understand if we are fixing an issue or if this is just a removal of a redundant signal. AFAICS we are only removing a `selectionChanged()` instance called by `clearSelection()` in "search mode", but this signal is also emitted in many other places (e.g. is emitted 3 times if I change selection with the mouse).
  >
  > Please explain the rationale for this patch in the commit message.
  
  
  Ping @meven

INLINE COMMENTS

> meven wrote in kitemlistcontroller.cpp:480
> This function way bugged : 
> The two branches of `if (searchFromNextItem)` both looked for the next keyboard with indexForKeyboardSearch(text, currentIndex (the first one with just a +1 modulo).
> But when searchFromNextItem is false, we are supposed to start to look for the next indexKeyboard from the start of the list `0`, not from the `currentIndex`.

I see. Please put this information in the commit message because it is valuable and it shouldn't be lost in a phabricator inline comment.

REPOSITORY
  R318 Dolphin

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

To: meven, elvisangelaccio, #dolphin
Cc: kfm-devel, iasensio, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20191020/45bc7af1/attachment.htm>


More information about the kfm-devel mailing list