Subject: Re: bug#8680: emacs 24.0 OS X keypad patch

From: Adrian Robert <[email-masked]>
Date: Wed, 6 Jul 2011 23:04:15 -0400

The code looks reasonable, but could the author explain why [theEvent keyCode] needs to be used in the keypad conversion instead of code?  The code would be more understandable if ns_convert_kaypad() and ns_convert_key() used the same argument.  Also this patch should be tested for correct behavior wrt this issue:

http://www.cocoabuilder.com/archive/cocoa/73306-workaround-for-broken-numlock-support.html#73306

thanks,
Adrian

On 2011/07/04, at 13:42, Stefan Monnier wrote:

> reassign 8680 emacs,ns
> tags 8680 +patch
> thanks
> 
> Could someone familiar with the MacOSX code take a look at this
> bug report?
> 
> 
>        Stefan
> 
> 
>>>>>> "Michael" == Michael Marchionna <[email-masked]> writes:
> 
>> Adding keypad keycodes to the existing translation table leads to some keys
>> being wrongly interpreted as keypad keys.  To avoid that, this patch
>> uses a separate translation table exclusively for keys that
>> generate a NSNumericPadKeyMask.
> 
> 
>> diff --git a/src/nsterm.m b/src/nsterm.m
>> index 91f0cbb..d537ee3 100644
>> --- a/src/nsterm.m
>> +++ b/src/nsterm.m
>> @@ -87,6 +87,7 @@ static unsigned convert_ns_to_X_keysym[] =
>>   NSBeginFunctionKey,           0x58,
>>   NSSelectFunctionKey,          0x60,
>>   NSPrintFunctionKey,           0x61,
>> +  NSClearLineFunctionKey,       0x0B,
>>   NSExecuteFunctionKey,         0x62,
>>   NSInsertFunctionKey,          0x63,
>>   NSUndoFunctionKey,            0x65,
>> @@ -134,6 +135,35 @@ static unsigned convert_ns_to_X_keysym[] =
>>   0x1B,				0x1B   /* escape */
>> };
> 
>> +static unsigned convert_nskeypad_to_X_keysym[] =
>> +{
>> +  /* Arrow keys are both function and keypad keys */
>> +  NSLeftArrowFunctionKey,       0x51,
>> +  NSUpArrowFunctionKey,         0x52,
>> +  NSRightArrowFunctionKey,      0x53,
>> +  NSDownArrowFunctionKey,       0x54,
>> +
>> +  0x41,                         0xAE,  /* KP_Decimal */
>> +  0x43,                         0xAA,  /* KP_Multiply */
>> +  0x45,                         0xAB,  /* KP_Add */
>> +  0x4B,                         0xAF,  /* KP_Divide */
>> +  0x4E,                         0xAD,  /* KP_Subtract */
>> +  0x51,                         0xBD,  /* KP_Equal */
>> +  0x52,                         0xB0,  /* KP_0 */
>> +  0x53,                         0xB1,  /* KP_1 */
>> +  0x54,                         0xB2,  /* KP_2 */
>> +  0x55,                         0xB3,  /* KP_3 */
>> +  0x56,                         0xB4,  /* KP_4 */
>> +  0x57,                         0xB5,  /* KP_5 */
>> +  0x58,                         0xB6,  /* KP_6 */
>> +  0x59,                         0xB7,  /* KP_7 */
>> +  0x5B,                         0xB8,  /* KP_8 */
>> +  0x5C,                         0xB9,  /* KP_9 */
>> +
>> +  // The enter key is on the keypad but modifier isnt set
>> +  NSEnterCharacter,		0x8D
>> +};
>> +
> 
>> static Lisp_Object Qmodifier_value;
>> Lisp_Object Qalt, Qcontrol, Qhyper, Qmeta, Qsuper, Qnone;
>> @@ -1924,13 +1954,33 @@ ns_convert_key (unsigned code)
>>   unsigned keysym;
>>   /* An array would be faster, but less easy to read. */
>>   for (keysym = 0; keysym < last_keysym; keysym += 2)
>> -    if (code == convert_ns_to_X_keysym[keysym])
>> -      return 0xFF00 | convert_ns_to_X_keysym[keysym+1];
>> +      
>> +      if (code == convert_ns_to_X_keysym[keysym]) {
>> +        return 0xFF00 | convert_ns_to_X_keysym[keysym+1];
>> +      }
>>   return 0;
>> /* if decide to use keyCode and Carbon table, use this line:
>>      return code > 0xff ? 0 : 0xFF00 | ns_keycode_to_xkeysym_table[code]; */
>> }
> 
>> +static unsigned
>> +ns_convert_keypad (unsigned code)
>> +/* --------------------------------------------------------------------------
>> +    Internal call used by NSView-keyDown.
>> +   -------------------------------------------------------------------------- */
>> +{
>> +  const unsigned last_keysym = (sizeof (convert_nskeypad_to_X_keysym)
>> +                                / sizeof (convert_nskeypad_to_X_keysym[0]));
>> +  unsigned keysym;
>> +  /* An array would be faster, but less easy to read. */
>> +  for (keysym = 0; keysym < last_keysym; keysym += 2) {
>> +      if (code == convert_nskeypad_to_X_keysym[keysym]) {
>> +        return 0xFF00 | convert_nskeypad_to_X_keysym[keysym+1];
>> +      }
>> +  }
>> +  return 0;
>> +}
>> +
> 
>> char *
>> x_get_keysym_name (int keysym)
>> @@ -4503,10 +4553,10 @@ ns_term_shutdown (int sig)
>>   Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (emacsframe);
>>   int code;
>>   unsigned fnKeysym = 0;
>> -  int flags;
>>   static NSMutableArray *nsEvArray;
>>   static BOOL firstTime = YES;
>>   int left_is_none;
>> +  unsigned int flags = [theEvent modifierFlags];
> 
>>   NSTRACE (keyDown);
> 
>> @@ -4550,9 +4600,13 @@ ns_term_shutdown (int sig)
>>       code = ([[theEvent charactersIgnoringModifiers] length] == 0) ?
>>         0 : [[theEvent charactersIgnoringModifiers] characterAtIndex: 0];
>>       /* (Carbon way: [theEvent keyCode]) */
>> +      
> 
>>       /* is it a "function key"? */
>> -      fnKeysym = ns_convert_key (code);
>> +      if (code < 0x00ff && (flags & NSNumericPadKeyMask) )
>> +	fnKeysym = ns_convert_keypad([theEvent keyCode]);
>> +      else
>> +	fnKeysym = ns_convert_key(code);
>>       if (fnKeysym)
>>         {
>>           /* COUNTERHACK: map 'Delete' on upper-right main KB to 'Backspace',
>> @@ -4565,7 +4619,6 @@ ns_term_shutdown (int sig)
> 
>>       /* are there modifiers? */
> emacs_event-> modifiers = 0;
>> -      flags = [theEvent modifierFlags];
> 
>>       if (flags & NSHelpKeyMask)
> emacs_event-> modifiers |= hyper_modifier;