Hotkey config #1571
Hotkey config #1571
Conversation
|
@Saviq what do you think about this? I am going a bit further than we discussed by tentatively placing things at the platform level, so that we can handle the mac case properly, rather than tell users they need to use ctrl for cmd and meta for ctrl. Refactoring needed to use current linux approach on win. |
This comment has been minimized.
This comment has been minimized.
|
How do you think they'll be inputting Command and Option? Cmd and Opt? |
|
I was thinking something like that. Not sure about Opt. Here's what they use: https://support.apple.com/en-us/HT201236 |
|
But I suppose we can accept both opt and alt. |
ricab
force-pushed
the
hotkey-config
branch
2 times, most recently
from
651719d
to
a0cd5ff
Compare
This comment has been minimized.
This comment has been minimized.
|
Tentative updates to the documentation in https://discourse.ubuntu.com/t/multipass-get-command/13735 and https://discourse.ubuntu.com/t/multipass-set-command/13734. Let me know if I am missing some other place where the hotkey is mentioned. |
|
I have found that both QKeySequence and QHotKey have a few rough corners. The altgr and meta issues are particularly unfortunate, because they don't work but there is no error whatsoever. The numpad issue too. The more I think about this, the more it feels like we should restrict the values to a subset that we know we can support well. Perhaps something like:
|
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## master #1571 +/- ##
==========================================
+ Coverage 75.23% 75.25% +0.02%
==========================================
Files 222 223 +1
Lines 8184 8204 +20
==========================================
+ Hits 6157 6174 +17
- Misses 2027 2030 +3
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
OK this should be about done now. Only need a final test on windows and probably some rebasing. |
This comment has been minimized.
This comment has been minimized.
| // A few notes on this: | ||
| // 1) Some shortcuts may feel counter-intuitive. For example in a keyboard where pressing "shift+-" produces an | ||
| // underscore, "_" is still interpreted the same as "-". IOW, "shift+-" == "shift+_" != "_" (just like "u" is the same | ||
| // as "U"). | ||
| // 2) QHotKey fails to register some of the shortcuts we accept here (e.g. "Media Play"). | ||
| // 3) QKeySequence seems to have problems with AtlGr. QKeySequence("AltGr").toString() prints rubbish (that it does not | ||
| // interpret back to mean the same thing). Unfortunately it is not enough to specify "ú" when that's what the layout | ||
| // produces for AltGr+U. The Sequence "ú" is accepted and QHotKey registers it, but is gets triggered on "U" and not | ||
| // "AltGr+U". | ||
| // 4) There does not seem to be a way to specify numpad keys in QKeySequence (with or without numlock). | ||
| // 5) meta only seems to work with other modifiers (e.g. ctrl+meta+x works, but meta+x doesn't even though it is | ||
| // accepted with no warning) |
There was a problem hiding this comment.
Does this apply to all platforms?
There was a problem hiding this comment.
I am not sure TBH, those were conclusions from testing on Linux. I got some of those same results on the others, but I did not check each one on every platform. The items referring to Meta and AltGr have doubtful applicability on macOS (at least with the default keyboard).
|
I wonder if we should display the hotkey in the GUI, then? |
Makes sense indeed. |
They may still be rejected for individual settings (currently the case for all settings).
(Still failing.)
(Makes new test pass.)
(Still failing.)
(Makes new tests pass.)
Creates a place for things that are used in more than one platform and which should be kept at the platform level (for dependency or conceptual reasons).
Use native representation of hotkeys for both presentation and persistence. This impacts macOS. With this approach, we can rely on Qt to 1) give us a correct (and pretty) representation to show the user; and 2) interpret our saved string correctly. We thus avoid a bunch of extra conversions like cmd <-> ctrl in macOS, keeping the general code for saving and retrieving settings clean.
Drop outdated TODOs.
This makes the backend file human readable when there are "funny" chars, but it does not affect logic, since the same format used is for reading and writing. Furthermore, no transitioning is needed because no existing settings accepted anything beyond ASCII, so they will be interpreted the same way in UTF-8.
The outcome is still platform dependent, but that's on Qt only.
|
macOS build available: multipass-1.4.0-dev.219.pr1571+gba14823e.mac-Darwin.pkg |
|
Alright, nice! bors merge |
|
Build failed: |
|
Conflict on macOS: |
Hi, that's because bors does not grab the matching branch when staging, right? |
Fixes #1311. Also introduces
get --raw.