Daniel P. Berrangé <berra...@redhat.com> writes:
> On Mon, May 23, 2022 at 09:23:48PM +0200, Thomas Huth wrote:
>> On 23/05/2022 15.45, Markus Armbruster wrote:
>> > Thomas Huth <th...@redhat.com> writes:
>> >
>> > > The "-display sdl" option still uses a hand-crafted parser for its
>> > > parameters since we didn't want to drag an interface we considered
>> > > somewhat flawed into the QAPI schema. Since the flaws are gone now,
>> > > it's time to QAPIfy.
>> > >
>> > > This introduces the new "DisplaySDL" QAPI struct that is used to hold
>> > > the parameters that are unique to the SDL display. The only specific
>> > > parameter is currently "grab-mod" that is used to specify the required
>> > > modifier keys to escape from the mouse grabbing mode.
>> > >
>> > > Signed-off-by: Thomas Huth <th...@redhat.com>
>> > > ---
>> > > qapi/ui.json | 26 ++++++++++++++-
>> > > include/sysemu/sysemu.h | 2 --
>> > > softmmu/globals.c | 2 --
>> > > softmmu/vl.c | 70 +----------------------------------------
>> > > ui/sdl2.c | 10 ++++++
>> > > 5 files changed, 36 insertions(+), 74 deletions(-)
>> > >
>> > > diff --git a/qapi/ui.json b/qapi/ui.json
>> > > index 11a827d10f..413371d5e8 100644
>> > > --- a/qapi/ui.json
>> > > +++ b/qapi/ui.json
>> > > @@ -1295,6 +1295,29 @@
>> > > '*swap-opt-cmd': 'bool'
>> > > } }
>> > > +##
>> > > +# @HotKeyMod:
>> > > +#
>> > > +# Set of modifier keys that need to be held for shortcut key actions.
>> > > +#
>> > > +# Since: 7.1
>> > > +##
>> > > +{ 'enum' : 'HotKeyMod',
>> > > + 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
>> >
>> > I have a somewhat uneasy feeling about encoding what is essentially a
>> > subset of the sets of modifier keys as an enumeration, but it's what we
>> > have to do to QAPIfy existing grab-mod.
>>
>> Well, that's exactly what you suggested here:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg03401.html
>>
>> So I really don't understand your uneasy feeling now?
I did mention the set of modifiers (encoded as list) design alternative.
You pointed out that you're merely QAPIfying what we have. Valid point,
well taken, I don't want to block that.
> I think we should consider what happens if we want to allow arbitrary
> key combinations in future, consisting of one or more modifiers together
> witha non-modifier key. For example CtrlL+ShiftL+F12. We won't want
> to be expressing the combinatorial expansion of all possible key
> combinations as an enum. Instead I think we would want to have an
> enum for keycode names and accept an array of them. We already
> have QKeyCode, so for SDL grab sequence we need to accept an
> arrray of QKeyCode.
>
> { 'struct' : 'DisplaySDL',
> 'data' : { '*grab-mod' : [ 'QKeyCode' ] }
>
> Good for QMP but not entirely pretty on the CLI. But we need back
> compat for existing CLI syntax too, so we would have to use an
> alternate allowing plain string for the legacy key codes combinations.
>
> IOW we end up needing
>
> { 'alternate': 'SDLGrabSeq',
> 'data': { 'grab-mod': 'HotKeyMod',
> 'grab-keys: [ 'QKeyCode' ] } }
>
> { 'struct' : 'DisplaySDL',
> 'data' : { '*grab-mod' : [ 'SDLGrabSeq' ] }
>
> deprecating 'grab-mod' for a while, eventually leaving just the
> first example above.
>
> Since IIUC, we can retrofit the alternate after the fact if we
> decide to allow arbitrary key combos, it means we could safely
> start with what Thomas proposes
>
> { 'struct' : 'DisplaySDL',
> 'data' : { '*grab-mod' : 'HotKeyMod' }
>
> and worry about the more general solution another day/month/year
Right.
[...]
