| Message ID | 20171214143713.3795-1-programmingkidx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20171214143713.3795-1-programmingkidx@gmail.com
Subject: [Qemu-devel] [PATCH v2] Add ability for user to specify mouse ungrab key
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
t [tag update] patchew/20171205055200.16305-1-peterx@redhat.com -> patchew/20171205055200.16305-1-peterx@redhat.com
t [tag update] patchew/20171205065307.21853-1-peterx@redhat.com -> patchew/20171205065307.21853-1-peterx@redhat.com
t [tag update] patchew/20171214124817.2567-1-dgilbert@redhat.com -> patchew/20171214124817.2567-1-dgilbert@redhat.com
* [new tag] patchew/20171214143713.3795-1-programmingkidx@gmail.com -> patchew/20171214143713.3795-1-programmingkidx@gmail.com
Switched to a new branch 'test'
829356df0b Add ability for user to specify mouse ungrab key
=== OUTPUT BEGIN ===
Checking PATCH 1/1: Add ability for user to specify mouse ungrab key...
ERROR: externs should be avoided in .c files
#145: FILE: vl.c:3143:
+int get_ungrab_key_value(void);
total: 1 errors, 0 warnings, 139 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
On Thu, Dec 14, 2017 at 09:37:13AM -0500, John Arbuckle wrote: > Currently the ungrab keys for the Cocoa and GTK interface are Control-Alt-g. > This combination may not be very fun for the user to have to enter, so we > now enable the user to specify their own key as the ungrab key. Since the > function keys are the keys that don't tend to be used that often and are > usually available, they will be the ones the user can pick to be the ungrab > key. We shouldn't make such assumptions about the function keys being what the users wants to use. We should allow any arbitrary combination of keys to be used. Just generalize the way Control+Alt+g is handled such that we match an arbitrary sequence instead of the hardcoded sequence. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > --- > v2 changes: > - Removed the "int i" code from the for loops. > > qemu-options.hx | 2 ++ > ui/cocoa.m | 20 ++++++++++++++-- > vl.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 92 insertions(+), 3 deletions(-) > > diff --git a/qemu-options.hx b/qemu-options.hx > index f11c4ac960..0a727ea50a 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -4472,6 +4472,8 @@ contents of @code{iv.b64} to the second secret > > ETEXI > > +DEF("ungrab", HAS_ARG, QEMU_OPTION_ungrab, \ > + "-ungrab <key>", QEMU_ARCH_ALL) Should be '<key sequence>', as we must allow more than one key to be listed to get a full sequence. eg you should be able to set -uncgrab ctrl+alt+g to replicate the default behaviour via CLI. For sanity, it is reasonable to limit the number of keys to max of 3. > diff --git a/ui/cocoa.m b/ui/cocoa.m > index 330ccebf90..8dc603adf0 100644 > --- a/ui/cocoa.m > +++ b/ui/cocoa.m > @@ -106,6 +106,9 @@ > NSTextField *pauseLabel; > NSArray * supportedImageFileTypes; > > +int get_ungrab_key_value(void); > +const char * get_ungrab_key_name(void); This shouldn't be in cocoa.m - we should have something in common code. Perhaps declare them in include/ui/console.h. Also give them a 'console_' prefix in fnuction name. Since we need a full sequence of keys we would need something more like int *console_ungrab_key_sequence() const char *console_ungrab_key_string() The first returns a 0 terminated array of ints, eg ctrl+alt+g would be represented as a 4 element array { Q_KEY_CODE_CTRL, Q_KEY_CODE_ALT, Q_KEY_CODE_G, 0 } > const int mac_to_qkeycode_map[] = { > [kVK_ANSI_A] = Q_KEY_CODE_A, > @@ -678,6 +681,12 @@ - (void) handleEvent:(NSEvent *)event > case NSEventTypeKeyDown: > keycode = cocoa_keycode_to_qemu([event keyCode]); > > + // if the user wants to release the mouse grab > + if (keycode == get_ungrab_key_value()) { > + [self ungrabMouse]; > + return; > + } This will have to track the sequence of pressed keys across invokations of handleEvent. This could be done by simple static variable that tracks the array index we've matched in the ungrab sequence. eg perhaps static size_t ungrab_index; int *ungrab_seq = get_ungrab_key_seq() if (keycode == ungrab_seq[ungrab_index]) { ungrab_index++; if (ungrab_seq[ungrab_seq] == 0) { [self ungrabMouse]; return; } } else { ungrab_index = 0; } this lets you remove the current code that deals with ctrl+alt+g ungrab. > diff --git a/vl.c b/vl.c > index 1ad1c04637..c2d848fabc 100644 > --- a/vl.c > +++ b/vl.c > @@ -185,7 +185,8 @@ bool boot_strict; > uint8_t *boot_splash_filedata; > size_t boot_splash_filedata_size; > uint8_t qemu_extra_params_fw[2]; > - > +int ungrab_key_value = -1; > +char *ungrab_key_name; > int icount_align_option; > > /* The bytes in qemu_uuid are in the order specified by RFC4122, _not_ in the > @@ -3088,6 +3089,73 @@ static void register_global_properties(MachineState *ms) > user_register_global_props(); > } > > +/* Sets the mouse ungrab key to what the user wants */ > +static void set_ungrab_key(const char *new_key) > +{ > + const char *keys[] = { > + [0 ... 0xff] = "", > + [Q_KEY_CODE_F1] = "f1", > + [Q_KEY_CODE_F2] = "f2", > + [Q_KEY_CODE_F3] = "f3", > + [Q_KEY_CODE_F4] = "f4", > + [Q_KEY_CODE_F5] = "f5", > + [Q_KEY_CODE_F6] = "f6", > + [Q_KEY_CODE_F7] = "f7", > + [Q_KEY_CODE_F8] = "f8", > + [Q_KEY_CODE_F9] = "f9", > + [Q_KEY_CODE_F10] = "f10", > + [Q_KEY_CODE_F11] = "f11", > + [Q_KEY_CODE_F12] = "f12", > + [Q_KEY_CODE_PRINT] = "f13", > + [Q_KEY_CODE_SCROLL_LOCK] = "f14", > + [Q_KEY_CODE_PAUSE] = "f15", > + }; We should just use the existing table generated by qapi. > + > + int key_value = -1; > + int i; > + > + /* see if the user's key is recognized */ > + for (i = 0; i < ARRAY_SIZE(keys); i++) { > + if (strcmp(keys[i], new_key) == 0) { > + key_value = i; > + break; > + } > + } And of course we need to cope with a full sequence > + > + /* if the user's key isn't recognized print the usable keys */ > + if (key_value == -1) { > + printf("Unrecognized key: %s\n", new_key); > + printf("Usable ungrab keys: "); > + for (i = 0; i < ARRAY_SIZE(keys); i++) { > + if (strlen(keys[i]) > 0) { /* filters out undefined values */ > + printf("%s ", keys[i]); > + } > + } > + printf("\n\n"); > + exit(1); > + } Not sure this is really needed - any valid qemu keycode should be allowed. > + > + ungrab_key_name = (char *) malloc(4 * sizeof(char)); > + strncpy(ungrab_key_name, new_key, 3); > + ungrab_key_value = key_value; > +} > + > +int get_ungrab_key_value(void); > + > +/* Returns the user specified ungrab key's value or -1 if not specified */ > +int get_ungrab_key_value(void) > +{ > + return ungrab_key_value; > +} > + > +const char *get_ungrab_key_name(void); > + > +/* Returns the name of the user specified ungrab key */ > +const char *get_ungrab_key_name(void) > +{ > + return ungrab_key_name; > +} Should all be in ui/console.c really, not vl.c Regards, Daniel
diff --git a/qemu-options.hx b/qemu-options.hx index f11c4ac960..0a727ea50a 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4472,6 +4472,8 @@ contents of @code{iv.b64} to the second secret ETEXI +DEF("ungrab", HAS_ARG, QEMU_OPTION_ungrab, \ + "-ungrab <key>", QEMU_ARCH_ALL) HXCOMM This is the last statement. Insert new options before this line! STEXI diff --git a/ui/cocoa.m b/ui/cocoa.m index 330ccebf90..8dc603adf0 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -106,6 +106,9 @@ NSTextField *pauseLabel; NSArray * supportedImageFileTypes; +int get_ungrab_key_value(void); +const char * get_ungrab_key_name(void); + // Mac to QKeyCode conversion const int mac_to_qkeycode_map[] = { [kVK_ANSI_A] = Q_KEY_CODE_A, @@ -678,6 +681,12 @@ - (void) handleEvent:(NSEvent *)event case NSEventTypeKeyDown: keycode = cocoa_keycode_to_qemu([event keyCode]); + // if the user wants to release the mouse grab + if (keycode == get_ungrab_key_value()) { + [self ungrabMouse]; + return; + } + // forward command key combos to the host UI unless the mouse is grabbed if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) { [NSApp sendEvent:event]; @@ -842,10 +851,17 @@ - (void) grabMouse COCOA_DEBUG("QemuCocoaView: grabMouse\n"); if (!isFullscreen) { + NSString * message_string; + if (get_ungrab_key_value() < 0) { + message_string = [NSString stringWithFormat: @"- (Press ctrl + alt + g to release Mouse"]; + } else { + message_string = [NSString stringWithFormat: @"- (Press ctrl + alt + g or %s to release Mouse", get_ungrab_key_name()]; + } + if (qemu_name) - [normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s - (Press ctrl + alt + g to release Mouse)", qemu_name]]; + [normalWindow setTitle:[NSString stringWithFormat: @"QEMU %s %@", qemu_name, message_string]]; else - [normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release Mouse)"]; + [normalWindow setTitle:[NSString stringWithFormat: @"QEMU %@", message_string]]; } [self hideCursor]; if (!isAbsoluteEnabled) { diff --git a/vl.c b/vl.c index 1ad1c04637..c2d848fabc 100644 --- a/vl.c +++ b/vl.c @@ -185,7 +185,8 @@ bool boot_strict; uint8_t *boot_splash_filedata; size_t boot_splash_filedata_size; uint8_t qemu_extra_params_fw[2]; - +int ungrab_key_value = -1; +char *ungrab_key_name; int icount_align_option; /* The bytes in qemu_uuid are in the order specified by RFC4122, _not_ in the @@ -3088,6 +3089,73 @@ static void register_global_properties(MachineState *ms) user_register_global_props(); } +/* Sets the mouse ungrab key to what the user wants */ +static void set_ungrab_key(const char *new_key) +{ + const char *keys[] = { + [0 ... 0xff] = "", + [Q_KEY_CODE_F1] = "f1", + [Q_KEY_CODE_F2] = "f2", + [Q_KEY_CODE_F3] = "f3", + [Q_KEY_CODE_F4] = "f4", + [Q_KEY_CODE_F5] = "f5", + [Q_KEY_CODE_F6] = "f6", + [Q_KEY_CODE_F7] = "f7", + [Q_KEY_CODE_F8] = "f8", + [Q_KEY_CODE_F9] = "f9", + [Q_KEY_CODE_F10] = "f10", + [Q_KEY_CODE_F11] = "f11", + [Q_KEY_CODE_F12] = "f12", + [Q_KEY_CODE_PRINT] = "f13", + [Q_KEY_CODE_SCROLL_LOCK] = "f14", + [Q_KEY_CODE_PAUSE] = "f15", + }; + + int key_value = -1; + int i; + + /* see if the user's key is recognized */ + for (i = 0; i < ARRAY_SIZE(keys); i++) { + if (strcmp(keys[i], new_key) == 0) { + key_value = i; + break; + } + } + + /* if the user's key isn't recognized print the usable keys */ + if (key_value == -1) { + printf("Unrecognized key: %s\n", new_key); + printf("Usable ungrab keys: "); + for (i = 0; i < ARRAY_SIZE(keys); i++) { + if (strlen(keys[i]) > 0) { /* filters out undefined values */ + printf("%s ", keys[i]); + } + } + printf("\n\n"); + exit(1); + } + + ungrab_key_name = (char *) malloc(4 * sizeof(char)); + strncpy(ungrab_key_name, new_key, 3); + ungrab_key_value = key_value; +} + +int get_ungrab_key_value(void); + +/* Returns the user specified ungrab key's value or -1 if not specified */ +int get_ungrab_key_value(void) +{ + return ungrab_key_value; +} + +const char *get_ungrab_key_name(void); + +/* Returns the name of the user specified ungrab key */ +const char *get_ungrab_key_name(void) +{ + return ungrab_key_name; +} + int main(int argc, char **argv, char **envp) { int i; @@ -4204,6 +4272,9 @@ int main(int argc, char **argv, char **envp) exit(1); } break; + case QEMU_OPTION_ungrab: + set_ungrab_key(optarg); + break; default: os_parse_cmd_args(popt->index, optarg); }
Currently the ungrab keys for the Cocoa and GTK interface are Control-Alt-g. This combination may not be very fun for the user to have to enter, so we now enable the user to specify their own key as the ungrab key. Since the function keys are the keys that don't tend to be used that often and are usually available, they will be the ones the user can pick to be the ungrab key. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- v2 changes: - Removed the "int i" code from the for loops. qemu-options.hx | 2 ++ ui/cocoa.m | 20 ++++++++++++++-- vl.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 3 deletions(-)