D10961: Add default shortcut "/" for opening filter panel

Nathaniel Graham noreply at phabricator.kde.org
Fri Mar 2 20:28:29 GMT 2018


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


  Works as advertised, and doesn't conflict with any default shortcuts. The whole "type / to quick search/filter" is pretty well established  in web browsers nowadays. Makes sense to have it in Dolphin, too.
  
  It might be nice to take the opportunity to modernize the syntax <https://doc.qt.io/qt-5.9/qkeysequence.html#details> for the existing shortcut here, replacing the `|` with a `+` for readability, since you're touching that code anyway.
  
  But now, to completely confuse you and undermine my credibility on the matter( ;-) ), let me recommend against including unrelated `#include` changes in this patch. There's a fine line to walk here, and personally, I think it's okay to modernize a piece of code that you happen to be touching, but it's best not to include unrelated code modernizations elsewhere in the file. Cleaning up those redundant `#includes` is a good idea, but should be separately, and preferably all at once, so it's easy to see if it causes any unexpected regressions.

REPOSITORY
  R318 Dolphin

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

To: rominf, #dolphin, ngraham
Cc: ngraham, #dolphin, spoorun, navarromorales, isidorov, firef, andrebarros, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180302/9e887607/attachment.htm>


More information about the kfm-devel mailing list