Use Ctrl+Shift+, as the standard shortcut for "Configure <Program>"
ClosedPublic

Authored by ngraham on Oct 14 2017, 2:46 PM.

Details

Summary

FEATURE: Use Ctrl+Shift+, as the standard keyboard shortcut to invoke KDE programs' "Configure <Program>" menu items.

Test Plan

This shortcut is not used by anything else. Searched on lxr, found one conflict in DigiKam, and the developers agreed to change it: https://bugs.kde.org/show_bug.cgi?id=386335

Will wait to land this until Digikam 5.8.0 is released to prevent any shortcut conflicts.

Tested in KDE Neon. Tried out Plasma, KWin, Dolphin, Kate, Konsole, Gwenview, Okular, Konversation, KTorrent, and Skanlite; all now have a consistent keyboard shortcut for their "Configure <Program>" menu items.

Diff Detail

Repository
R237 KConfig
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
There are a very large number of changes, so older changes are hidden. Show Older Changes

Agree with Kai - this needs good testing that it is not used anywhere. Best by looking through lxr which apps use this framework.

Sounds good. What's lxr?

Sounds good. What's lxr?

lxr.kde.org

Unless I'm holding it wrong, https://lxr.kde.org/ident?_i=Comma&_remember=1 doesn't appear show any other uses of Ctrl+, as a keyboard shortcut.

ngraham edited the test plan for this revision. (Show Details)Oct 14 2017, 5:27 PM

Any remaining concerns?

marten added a subscriber: marten.EditedOct 15 2017, 8:31 AM

FYI: KMail uses Ctrl+comma and Ctrl+dot as standard shortcuts for "Collapse all threads" and "Expand all threads" respectively.

Try https://lxr.kde.org/ident?_i=Key_Comma&_remember=1

kfunk added a subscriber: kfunk.Oct 15 2017, 8:52 AM

In KDevelop, this is also a really important shortcut for navigating through the source. Ctrl+, for jumping to definition, Ctrl+. for jumping to declaration. /me wonders if one really needs a shortcut for configuring shortcuts at all.

https://lxr.kde.org/search?_filestring=&_string=Qt%3A%3ACTRL.*Qt%3A%3AKey_Comma lists the usages (=> Digikam, KDevelop, KMail)

In D8296, @ngraham wrote:

I chose Ctrl+, because that's the standard shortcut for all (and I do mean all) programs in the macOS world--a standard that has started to trickle into the Windows world as well, because there was previously no standard there. Might as well standardize on what's already a de facto standard elsewhere, to avoid throwing away people's muscle memory.

The muscle memory for Mac users isn't preserved, as the keystroke there is command-, not control, and command physically corresponds to Alt. So this only gets us a relatively new Windows shortcut, on top of the legacy CUA system. I have no idea whether Windows still follows CUA or not.

In D8296#155573, @kfunk wrote:

In KDevelop, this is also a really important shortcut for navigating through the source. Ctrl+, for jumping to definition, Ctrl+. for jumping to declaration. /me wonders if one really needs a shortcut for configuring shortcuts at all.

As we're a CUA-derived UI, we already have a keyboard shortcut for Configure Shortcuts: Alt-s Alt-h. Same for Configure $app: Alt-s Alt-c

While I do find switching back and forth between Mac and CUA intermittently frustrating, we really can't easily implement the same suite of keystrokes as Mac because of the Alt combinations being reserved for menus.

Darn, that's a shame. Looks like I was holding it wrong. I'll find another shortcut that doesn't conflict. Any suggestions?

ngraham updated this revision to Diff 20821.Oct 15 2017, 8:39 PM

lxr.kde.org showed several programs using Ctrl+, but nothing uses Ctrl+Alt+,

ngraham retitled this revision from Use Ctrl+, as the standard shortcut for "Configure <Program>" to Use Ctrl+Alt+, as the standard shortcut for "Configure <Program>".Oct 15 2017, 8:41 PM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
aacid added a subscriber: aacid.Oct 15 2017, 10:10 PM

do you really find yourself opening the app settings that offen that you need a shortcut?

I personally don't think adding this shortcut makes sense, the preferences is not something you invoke usually, i.e. like Ctrl+C, Ctrl+V

Your original patch had some value trying to cater for the macOs refugees, but now that isn't even the case, i have to ask, why "steal" from apps one more shortcut?

I do find myself doing this a lot, yes. I know a lot of people who, upon downloading a new piece of software, the first thing they do is check out the preferences window to see what's available. It's a subtle thing, but throughout the macOS world where the shortcut is consistent across all apps, people do in fact open the Preferences windows a lot. With a consistent shortcut, it's practically a zero-effort operation. In platforms without that, the mental and physical effort is greater, so they don't open the Preferences window as much.

Ctrl-Alt-, is way outside of anything I would expect as a common shortcut. Did you try Alt-,? I have no idea whether it'd work or not, but the odds of having a menu whose name begins with (or even contains) a comma are low.

ngraham updated this revision to Diff 20839.Oct 16 2017, 3:10 AM

Switch to Alt+, since that makes a bit more sense and actually preserves recovering Mac users' muscle memory (Alt being in the same place as the Command key)

ngraham retitled this revision from Use Ctrl+Alt+, as the standard shortcut for "Configure <Program>" to Use Alt+, as the standard shortcut for "Configure <Program>".Oct 16 2017, 3:12 AM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)

Alt+, is definitely better, yeah. Also, now we are actually preserving recovering Mac users' muscle memory, since the Alt key on a PC keyboard is in the same place as the Command key on a Mac keyboard.

Any objections to Alt+, ?

If there are no objections, can someone grudgingly mark it as approved? ;-)

elvisangelaccio added inline comments.
src/gui/kstandardshortcut.cpp
159

I'd use ALT(Comma) here, for consistency with the other shortcuts.

ngraham updated this revision to Diff 21142.Oct 22 2017, 9:17 PM

Use syntax that's consistent with the rest of the file

ngraham marked an inline comment as done.Oct 22 2017, 9:18 PM

Since @ngraham added me as reviewer, here are my 2 cents. Trying the patch, I made those observations:

  • Alt often is released by the user only after the dialog is shown already, resulting in the (normally hidden) accelerators ("_") of the dialog showing and then dissappearing, which looks odd.
  • Normally Alt is used either for accessing the menu itself or in combination with the cursor/navigation keys, but not so much for shortcuts. While there are exceptions, at least our shortcuts for executing standard actions affecting all apps should IMHO stick to Ctrl.
  • Does not work in Kontact and KMail (but does work in KOrganizer and KMail's "New Mail" window).

Note I don't oppose the idea to have a universal shortcut for opening the preferences, but I feel that with the currently selected shortcut the net effect to the overall user experience and consistency is slightly negative.

I won't block this patch, but instead let me add this ideas:

  • The patch is not really urgent. Do we have the chance to gather some telemetry data on the usage of shortcuts as well as on the preferences dialog? That way we could better evaluate the usefulness and maybe even choose non-conflicting shortcuts.
  • Apply the change only via a shortcut theme package for Mac refugees (not sure if can we have such a thing?)
  • If we change it at all, this should get in very early in the cycle to judge how much this conflicts with custom shortcuts users have.

Thanks for your comments, @rkflx. I have no strong opinions on what the shortcut should be.

Side note: if KMail and Kontact don't get this change automatically, that's a sign that they're not using KStandardAction::Preferences() and should be.

abetts added a subscriber: abetts.Oct 25 2017, 11:55 PM

Could this also be added to global shortcuts if it is not there already? That the user can define the keys to use for Preferences?

ngraham planned changes to this revision.Oct 25 2017, 11:57 PM

Oh, great idea.

Just checked: It's already there in Standard Shortcuts, where it can be changed.

ngraham edited the summary of this revision. (Show Details)Oct 29 2017, 2:50 AM
ngraham retitled this revision from Use Alt+, as the standard shortcut for "Configure <Program>" to Use Ctrl+Alt+, as the standard shortcut for "Configure <Program>".Oct 29 2017, 2:58 AM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
ngraham updated this revision to Diff 21507.Oct 29 2017, 3:00 AM

Changed to Ctrl+Alt+, because Alt-only shortcuts are weird

rkflx added a comment.Oct 29 2017, 7:26 AM

Just checked: It's already there in Standard Shortcuts, where it can be changed.

Well, it says Configure System Settings…, so I even discarded this when first looking for it. As a soon-to-be shortcut expert, do you see a chance this could be changed to Configure Application… (only in System Settings, of course)?

Changed to Ctrl+Alt+, because Alt-only shortcuts are weird

Still feels a bit awkward and has the Alt accelerator problem. Maybe something involving Ctrl+? (← that's a "Shift")

In D8296#161367, @rkflx wrote:

Just checked: It's already there in Standard Shortcuts, where it can be changed.

Well, it says Configure System Settings…, so I even discarded this when first looking for it. As a soon-to-be shortcut expert, do you see a chance this could be changed to Configure Application… (only in System Settings, of course)?

Definitely, that bugs me too. Now to figure out where exactly this is set...

In D8296#161367, @rkflx wrote:

Still feels a bit awkward and has the Alt accelerator problem. Maybe something involving Ctrl+? (← that's a "Shift")

Yeah that's probably for the Best. Too bad Ctrl+Shift+, doesn't work only because Digikam is using it. I'll to find something else.

What about Windows/Meta + Something, let's say , comma?

That would work, and it's not taken, but I worry that this wouldn't help a lot of users, because outside of seasoned Linux users, Meta isn't a well known description for the Windows/super/Command key (depending on the keyboard hardware).

If you assume that most computers that run linux will have a windows key, I would call it just windows. Everyone else uses exceptions really.

rkflx added a comment.Oct 29 2017, 5:23 PM

Is there any other menu action which has Meta in its shortcut? If not, this might be an indication that we are heading in the wrong direction. I find Meta more appropriate for global shortcuts (virtual desktops, activities, window management, Kickoff, …), where it is already used. Do we have a HIG regarding this, so we maintain at least some order and logic?

I agree, @rkflx. Shame Ctrl+Shift+, is taken.

Ctrl+Shift+\, maybe? That's not taken.

rkflx added a comment.Oct 29 2017, 5:30 PM

Try typing that on a non-US keyboard layout :)

We don't seem to have anything against it. My only request is that if we do shortcuts, make them use 2 hands instead of just one. If the keys are too close together, this makes it harder to enter. Possibly harder for people with disabilities as well. So, space the sets in 2, one on the left side of the keyboard and the next symbol on the right.

Shortcuts HIG

rkflx added a comment.Oct 29 2017, 5:37 PM

Well, that's more of a list than a well-explained guideline. This is quite good (grep for "system reserved shortcuts", but also see general reasoning): https://developer.gnome.org/hig/stable/keyboard-input.html.en

ngraham updated this revision to Diff 21530.Oct 29 2017, 5:38 PM

Amazingly, Ctrl+Shift+. isn't taken!

ngraham retitled this revision from Use Ctrl+Alt+, as the standard shortcut for "Configure <Program>" to Use Ctrl+Shift+. as the standard shortcut for "Configure <Program>".Oct 29 2017, 5:38 PM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
ngraham updated this revision to Diff 21531.Oct 29 2017, 5:54 PM

Improved translation string to provide a better hint about what this really is

So as for changing the name...
I notice that this uses QT_TRANSLATE_NOOP3() instead of i18n. It seems that the English string at least is simply a bad translation, likely caused by the poorly-chosen base string, which I've changed to be a bit more descriptive. I've never done any translation work before; can anyone give me a super-quick primer on how to submit a better string here?

rkflx added a subscriber: ilic.Oct 29 2017, 6:04 PM

Was confused and it took me a while to figure out how this will end up for different keyboard layouts. Turns out using the KCM combines +key, while arc patch results in all three keys shown in the menu.

This means that . is always used, regardless of keyboard layout. As this key is quite important for writing, it should be easy to access almost everywhere.

Great find, I like it :) (and it even fulfills @abetts' demands for two hands). Let's hope nobody finds any obscure place where this is used already...

can anyone give me a super-quick primer on how to submit a better string here?

Last time we summoned @ilic it worked out well, otherwise do some API docs reading?

rkflx added a comment.Oct 29 2017, 6:13 PM
In D8296#161557, @rkflx wrote:

Let's hope nobody finds any obscure place where this is used already...

Hm, conflicts in Konsole. However, can we change Konsole? At least, you could use this to improve your lxr search, so maybe more is found (bad) or Konsole is the only place (not quite as bad).

ngraham planned changes to this revision.Oct 29 2017, 6:16 PM

Urgh, didn't find it because it's defined in Konsole like this:

Konsole::ACCEL + Qt::SHIFT + Qt::Key_Period

ilic added a comment.Oct 29 2017, 6:26 PM

"Configure Application" does sound better than "Preferences", but that's just my opinion as a random translator, no link to my i18n plumber's hat :)

ngraham added a comment.EditedOct 29 2017, 6:33 PM

@rkflx
If we need to change one app, we could also target Digikam, which is as far as I can tell the only one using Ctrl+Shift+,

@ilic
Actually the problem is that this string--whatever it is in the code--is being changed for English to "Configure System Settings..." when viewed in System Settings > Shortcuts > Standard Shortcuts:

I haven't been able to find where that's being done. Also, is it a problem that this is using QT_TRANSLATE_NOOP3() instead of i18n()?

rkflx added a comment.Oct 29 2017, 6:58 PM

Digikam, which is as far as I can tell the only one using Ctrl+Shift+,

Is it, though? I don't see it here:

There might actually be a bug here: Adapting your patch to Ctrl++,, I get this for Digikam:

  • warning on startup
  • Configure Digikam works
  • Zoom to 100% works
  • shortcuts listed as Ctrl++, and Ctrl+,

Possibly the conflict detection is broken?

rkflx added a comment.Oct 29 2017, 7:46 PM
In D8296#161570, @rkflx wrote:

Possibly the conflict detection is broken?

Turns out it's not, the shortcut is actually defined for the "Light Table". However, there the shortcut is listed twice and works the same (for me at least). Might be worth talking to Digikam if this is an oversight or what is going on there.

kfunk removed a subscriber: kfunk.Oct 29 2017, 7:49 PM
ilic added a comment.Oct 29 2017, 7:54 PM

KStandardShortcut::Preferences label is being overridden in KConfigWidgets's src/kstandardaction_p.h to "&Configure %1..." and %1 substituted with current application name.

In D8296#161574, @rkflx wrote:
In D8296#161570, @rkflx wrote:

Possibly the conflict detection is broken?

Turns out it's not, the shortcut is actually defined for the "Light Table". However, there the shortcut is listed twice and works the same (for me at least). Might be worth talking to Digikam if this is an oversight or what is going on there.

Yeah, I've filed https://bugs.kde.org/show_bug.cgi?id=386335

In D8296#161576, @ilic wrote:

KStandardShortcut::Preferences label is being overridden in KConfigWidgets's src/kstandardaction_p.h to "&Configure %1..." and %1 substituted with current application name.

Hah! No wonder I couldn't find the string anywhere. Looks like a special case will be needed.

ngraham updated this revision to Diff 21542.Oct 30 2017, 12:52 AM

Ctrl+Shift+Comma FTW

ngraham retitled this revision from Use Ctrl+Shift+. as the standard shortcut for "Configure <Program>" to Use Ctrl+Shift+, as the standard shortcut for "Configure <Program>".Oct 30 2017, 12:54 AM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)Oct 30 2017, 12:54 PM

The DigiKam folks changed the shortcut, so we can use Ctrl+Shift+, now! Any remaining objections?

rkflx accepted this revision.Oct 31 2017, 1:26 PM

As mentioned in the bug, we should wait until after digiKam has had a release containing the change, i.e. 5.8.0. Provoking the dreaded shortcut warning dialog would be a bad way to say "thank you". Roadmap and NEWS say the release was planned for 2017-10-29, but it is still not tagged. We can wait a couple of days, otherwise I assume Frameworks 5.41 (or later, no need to rush) would fit better.

That said, more reviewers looking for potential additional conflicts would still be appreciated.

This revision is now accepted and ready to land.Oct 31 2017, 1:26 PM

Makes sense!

ngraham edited the summary of this revision. (Show Details)Nov 10 2017, 6:37 AM
ngraham edited the test plan for this revision. (Show Details)
In D8296#162352, @rkflx wrote:

As mentioned in the bug, we should wait until after digiKam has had a release containing the change, i.e. 5.8.0. Provoking the dreaded shortcut warning dialog would be a bad way to say "thank you". Roadmap and NEWS say the release was planned for 2017-10-29, but it is still not tagged.

Reading https://mail.kde.org/pipermail/digikam-devel/2017-December/100914.html, we don't have to wait very long any more ;)

Looks like DigiKam 5.8.0 is supposed to be released in two days: https://staging.digikam.org/news/2018-01-14-5.8.0_release_announcement/

Once it's released, I'll land this.

This revision was automatically updated to reflect the committed changes.

Landed now that Digikam 5.8.0 has been released, and updated the wiki: https://community.kde.org/KDE_Visual_Design_Group/HIG/Keyboard_Shortcuts

rkflx added a comment.Mar 4 2018, 7:14 AM

@ngraham Is (K)ubuntu 18.04 really going to ship Digikam 5.6 from June 2017, i.e. creating an influx of bug reports on shortcut warnings? Can this be patched in either Digikam or KF5 packages?

@rkflx I'm looking into it now.