Add default shortcut "/" for opening filter panel.
FEATURE: 156381
ngraham |
Dolphin |
Add default shortcut "/" for opening filter panel.
FEATURE: 156381
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
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.
Konsole still eats it when its panel is open, but it also eats the other ctrl+I shortcut even without this patch, so that seems like another bug (and down the rabbit hole we go...)
It might be nice to take the opportunity to modernize the syntax 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.
Done
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.
OK. Reverted those changes. Do you mind if I do massive commit (for the whole project) that cleans up the code?
As long as it's in the form of a reviewable Phabricator patch, I think that would be very welcome! Various other KDE projects (Baloo, KDE Connect) are also undergoing code clean-up and refactoring, in fact.
Since you now have a shiny new new contributor account, would you like to land the patch on Dolphin's master branch?
https://community.kde.org/Infrastructure/Phabricator#Step_3:_Land_your_diff