BUG: 383603
Add the new KDE standard tab switching shortcuts that were introduced in https://phabricator.kde.org/R237:d85a57645f94d2d087711d7c608a0c5a46ed7ede.
hindenburg | |
aacid | |
ltoscano | |
pino | |
dfaure |
KDE Applications | |
Konsole |
BUG: 383603
Add the new KDE standard tab switching shortcuts that were introduced in https://phabricator.kde.org/R237:d85a57645f94d2d087711d7c608a0c5a46ed7ede.
Built and tested with current git master in up-to-date KDE Neon. Expected behavior is seen (default shortcuts have become shift+left/right and cmd+pgup/pgdn).
Lint Skipped |
Unit Tests Skipped |
I understand your concern, Albert. But it seems that the decision to change the tab switching shortcuts throughout all KDE apps has already been made (not by me), in https://phabricator.kde.org/R237:d85a57645f94d2d087711d7c608a0c5a46ed7ede.
All this does is make Konsole consistent with the new shortcut, because Konsole doesn't implement KStandardShortcut::tabPrev() and KStandardShortcut::tabNext(). If it did, then it would have gotten up the change automatically, no new commits required.
Anyway these shortcuts are all user-changeable, so it's not like in the GTK world where what we decide to do may irritate some users forever with no recourse.
I already complained there, and will complain here too.
Anyhow there's magic that allows for two shortcuts, if you really need Ctrl based shortcuts, you can add them as secondary shortcuts and not break people that are used to Shift.
Good point. It looks like setDefaultShortcut() can take a QList of shortcuts instead of just a single one, which will add the new shortcut without blowing away the current one. This works since Konsole currently has no secondary shortcut, so we wouldn't be breaking anybody's secondary shortcut muscle memory.
Konsole is a special case when it comes to shortcuts.
Terminal apps use ctrl+whatever themselves and obviously konsole can't use the same.
For example if copy was control+c it would conflict with being able to exit an app.
That's true, but ctrl-pgup/pgdn don't emit any special signals, do they? I proposed this change only because it doesn't seem to conflict with anything (I wouldn't have done it if we decided on ctrl-c or ctrl-x for tab switching :) ).
+1 from me, the change as it is now is pretty harmless (I mean, not dangerous), since it doesn't remove any previously known shortcut, and I find it extremely unlikely that a text console application would be using Ctrl+PgUp/PgDown itself.
src/ViewManager.cpp | ||
---|---|---|
253–254 | could be written in a much shorter way: const QList<QKeySequence> previousViewActionKeys{Qt::SHIFT + Qt::Key_Left, Qt::CTRL + Qt::KeyPageUp}; |
I don't have any strong objection - I didn't realize browsers use this shortcut - please use David's shorter code
Thanks David! Do I need do do anything else, or will someone else commit this? I don't have commit access.