Re: [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode

[Programmingkid]

On Mar 1, 2016, at 6:16 PM, Eric Blake wrote:

> On 03/01/2016 03:12 PM, Programmingkid wrote:
>> The old pc/xt keyboard keycode array is replaced with QEMU's own QKeyCode
>> layout.
>> 
>> Signed-off-by: John Arbuckle <address@hidden>
>> 
>> ---
>> Maintainer note:
>> Please apply these patches before testing: 
>> - qapi-schema.json: Add kp_equals and power keys
>> - adb.c: Replace pc_to_adb_keycode with more detailed array
>> - MacKeys.h: initial commit
> 
> Since this depends on those patches, it might have been nicer to submit
> it as a series with a 0/4 cover letter, rather than making us chase
> separate threads.

Learning how to use git send-email is now on my to-do list.

> 
>> +
>> +    [MAC_KEY_ESC] = Q_KEY_CODE_ESC,
>> +    //[MAC_KEY_F1] = Q_KEY_CODE_POWER, // Just in case you need the power 
>> key
>> +    [MAC_KEY_F1] = Q_KEY_CODE_F1,
> 
> The comment looks weird. Probably worth a mention in the commit message
> why you need it.

Maybe this:

[MAC_KEY_ESC] = Q_KEY_CODE_ESC,
//[MAC_KEY_F1] = Q_KEY_CODE_POWER, // You might need this key if you use 
Macsbug. 
[MAC_KEY_F1] = Q_KEY_CODE_F1,


> 
>> 
>> static int cocoa_keycode_to_qemu(int keycode)
>> {
>> -    if (ARRAY_SIZE(keymap) <= keycode) {
>> +    if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
>>         fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode);
> 
> Pre-existing, but we should fix this to avoid fprintf.

What do you mean by pre-existing? I personally don't have anything against 
fprintf, but switching to printf is just fine with me.