[PATCH] Enhanced hotkey handling for menuentry

[PATCH] Enhanced hotkey handling for menuentry

[Andreas Vogel]

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

Here is a small patch which enhances the hotkey handling for menu entries.
        - new static function parse_key()
        - hotkey aliases are now case insensitive
        - additional hotkey aliases
        - handling now <SHIFT> and <CTRL> modifiers for hotkeys

Any chance that this is included (Vladimir: taking into account that
might do some work in these area anyway)? Any comments?

Andreas

[-- Attachment #2: 01-menuentry_enhanced_hotkeys.patch --]
[-- Type: text/plain, Size: 4795 bytes --]

------------------------------------------------------------
revno: 4098
committer: Andreas Vogel <Andreas.F.Vogel@gmail.com>
branch nick: 01-menuentry_enhanced_hotkeys
timestamp: Thu 2012-03-01 22:38:32 +0100
message:
  	* grub-core/commands/menuentry.c: enhanced hotkey handling:
  	  - new static function parse_key()
  	  - hotkey aliases are now case insensitive
  	  - additional hotkey aliases
  	  - handling now <SHIFT> and <CTRL> modifiers for hotkeys
diff:
=== modified file 'grub-core/commands/menuentry.c'
--- grub-core/commands/menuentry.c	2012-02-26 16:28:05 +0000
+++ grub-core/commands/menuentry.c	2012-03-01 21:38:32 +0000
@@ -45,24 +45,110 @@
   int key;
 } hotkey_aliases[] =
   {
-    {"backspace", '\b'},
-    {"tab", '\t'},
-    {"delete", GRUB_TERM_KEY_DC},
-    {"insert", GRUB_TERM_KEY_INSERT},
-    {"f1", GRUB_TERM_KEY_F1},
-    {"f2", GRUB_TERM_KEY_F2},
-    {"f3", GRUB_TERM_KEY_F3},
-    {"f4", GRUB_TERM_KEY_F4},
-    {"f5", GRUB_TERM_KEY_F5},
-    {"f6", GRUB_TERM_KEY_F6},
-    {"f7", GRUB_TERM_KEY_F7},
-    {"f8", GRUB_TERM_KEY_F8},
-    {"f9", GRUB_TERM_KEY_F9},
-    {"f10", GRUB_TERM_KEY_F10},
-    {"f11", GRUB_TERM_KEY_F11},
-    {"f12", GRUB_TERM_KEY_F12},
+    { "backspace", '\b'},
+    { "tab",       '\t'},
+    { "delete",    GRUB_TERM_KEY_DC },
+    { "insert",    GRUB_TERM_KEY_INSERT },
+    { "up",        GRUB_TERM_KEY_UP },
+    { "down",      GRUB_TERM_KEY_DOWN },
+    { "left",      GRUB_TERM_KEY_LEFT },
+    { "right",     GRUB_TERM_KEY_RIGHT },
+    { "prev",      GRUB_TERM_KEY_PPAGE },
+    { "next",      GRUB_TERM_KEY_NPAGE },
+    { "home",      GRUB_TERM_KEY_HOME },
+    { "end",       GRUB_TERM_KEY_END },
+    { "f1",        GRUB_TERM_KEY_F1 },
+    { "f2",        GRUB_TERM_KEY_F2 },
+    { "f3",        GRUB_TERM_KEY_F3 },
+    { "f4",        GRUB_TERM_KEY_F4 },
+    { "f5",        GRUB_TERM_KEY_F5 },
+    { "f6",        GRUB_TERM_KEY_F6 },
+    { "f7",        GRUB_TERM_KEY_F7 },
+    { "f8",        GRUB_TERM_KEY_F8 },
+    { "f9",        GRUB_TERM_KEY_F9 },
+    { "f10",       GRUB_TERM_KEY_F10 },
+    { "f11",       GRUB_TERM_KEY_F11 },
+    { "f12",       GRUB_TERM_KEY_F12 },
   };
 
+
+/* Parse the given string and return a GRUB_TERM key.
+   The given string can be one of the predefined key names (aliases),
+   otherwise the first character of the given str is taken as key.
+   Modifiers like "<SHIFT>" or "<CTRL>" are handled. They may appear
+   in any order but need to be in front of the alias or key.
+   Spaces between the modifiers and key are silently ignored.
+   Comparison with alias names is case independent. */
+
+static int
+parse_key (const char *string)
+{
+  char *str, *ptr, *keystr;
+  int key = 0;
+  int i;
+
+  if (string == 0 || *string == 0)
+    return 0;
+
+  str = grub_strdup (string);
+  if (!str)
+    return 0;
+
+#define STRING_CTRL  "<CTRL>"
+#define STRING_SHIFT "<SHIFT>"
+
+  /* check for CTRL modifier */
+  ptr = grub_strstr (str, STRING_CTRL);
+  if (ptr)
+  {
+    /* replace modifier string with spaces */
+    for (i = 0; i < grub_strlen (STRING_CTRL); i++)
+      *(ptr + i) = ' ';
+    key |= GRUB_TERM_CTRL;
+  }
+
+  /* check for SHIFT modifier */
+  ptr = grub_strstr (str, STRING_SHIFT);
+  if (ptr)
+  {
+    /* replace modifier string with spaces */
+    for (i = 0; i < grub_strlen (STRING_SHIFT); i++)
+      *(ptr + i) = ' ';
+    key |= GRUB_TERM_SHIFT;
+  }
+
+  /* remove/zero trailing spaces */
+  for (ptr = str + grub_strlen (str) - 1; ptr >= str && *ptr == ' '; ptr--)
+  *ptr = 0;
+
+  /* skip leading spaces */
+  for (ptr = str; *ptr == ' '; ptr++) /*NOTHING*/;
+
+  /* the remaining chars are the real key string */
+  keystr = ptr;
+
+  /* check if the key string is an alias name */
+  for (i = 0; i < ARRAY_SIZE (hotkey_aliases); i++)
+  {
+    /* case is not significant for key aliases */
+    if (grub_strcasecmp (keystr, hotkey_aliases[i].name) == 0)
+    {
+      key |= hotkey_aliases[i].key;
+      break;
+    }
+  }
+
+  if (i == ARRAY_SIZE (hotkey_aliases))
+  {
+    /* no alias matched, so use the first char in the given string as the key */
+    key |= keystr[0];
+  }
+
+  grub_free (str);
+
+  return key;
+}
+
 /* Add a menu entry to the current menu context (as given by the environment
    variable data slot `menu').  As the configuration file is read, the script
    parser calls this when a menu entry is to be created.  */
@@ -117,17 +203,7 @@
     }
 
   if (hotkey)
-    {
-      unsigned i;
-      for (i = 0; i < ARRAY_SIZE (hotkey_aliases); i++)
-	if (grub_strcmp (hotkey, hotkey_aliases[i].name) == 0)
-	  {
-	    menu_hotkey = hotkey_aliases[i].key;
-	    break;
-	  }
-      if (i == ARRAY_SIZE (hotkey_aliases))
-	menu_hotkey = hotkey[0];
-    }
+    menu_hotkey = parse_key (hotkey);
 
   if (! argc)
     {

Re: [PATCH] Enhanced hotkey handling for menuentry

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 03.03.2012 21:43, Andreas Vogel wrote:
> Here is a small patch which enhances the hotkey handling for menu entries.
Could you use GNU style? You miss the spaces before braces. Comments 
have to be full sentences, start with uppercase and end with a dot 
followed by 2 spaces.
Comment /*NOTHING*/ is not useful.
>          - new static function parse_key()
>          - hotkey aliases are now case insensitive
>          - additional hotkey aliases
All added aliases conflict with normal function of these keys (arrows 
and pages)
>          - handling now<SHIFT>  and<CTRL>  modifiers for hotkeys
Could you change to the emacs notation? Shift flag isn't valid with 
alphanumeric keys.




-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: [PATCH] Enhanced hotkey handling for menuentry

[Andreas Vogel]

> On 03.03.2012 21:43, Andreas Vogel wrote:
>> Here is a small patch which enhances the hotkey handling for menu
>> entries.
> Could you use GNU style? You miss the spaces before braces.
Sure. Missed by accident.

> Comments have to be full sentences, start with uppercase and end with
> a dot followed by 2 spaces.
Wow... ok.

> Comment /*NOTHING*/ is not useful.
I disagree, because it's more clear that this is an intentionally left
empty body. But it's a matter of taste and I obey.

>>          - new static function parse_key()
>>          - hotkey aliases are now case insensitive
>>          - additional hotkey aliases
> All added aliases conflict with normal function of these keys (arrows
> and pages)
Yes, I know. But it doesn't harm anyway and it might be useful to have
the code already in case the use of the keys change in the future.
Anyway, if you insist of leaving them out i'll obey.

>>          - handling now<SHIFT>  and<CTRL>  modifiers for hotkeys
> Could you change to the emacs notation?
What is the emacs notation? I have no idea.

> Shift flag isn't valid with alphanumeric keys.
Ah, ok. Am I right that it's enough just not to set the SHIFT mask in
case isalnum() is true?

Re: [PATCH] Enhanced hotkey handling for menuentry

[Andreas Vogel]

Am 03.03.2012 22:30, schrieb Andreas Vogel:
>>>          - new static function parse_key()
>>>          - hotkey aliases are now case insensitive
>>>          - additional hotkey aliases
>> All added aliases conflict with normal function of these keys (arrows
>> and pages)
> Yes, I know. But it doesn't harm anyway and it might be useful to have
> the code already in case the use of the keys change in the future.
> Anyway, if you insist of leaving them out i'll obey.
Thought about it again: when being now able to use modifiers ALT, CTRL
and SHIFT, using the keys which have a already defined function (like
arrows and pages) doesn't conflict. Even though e.g. KEY_UP is already
used in menu I would like to be able to use SHIFT-UP as a hotkey. So in
my opinion the hotkey parse function should handle all possible keys.

>>>          - handling now<SHIFT>  and<CTRL>  modifiers for hotkeys
>> Could you change to the emacs notation?
> What is the emacs notation? I have no idea.
I did a search and from what I've seen I assume you mean to use the
following:
    - "C-" for CTRL
    - "A-" for ALT (even though emacs seems to use "M-" for ALT on most
system)
    - "S-" for SHIFT (emacs doesn't seem to have any notion for SHIFT,
but here we would need it)
Using this notion valid hotkeys could be e.g. "C-F1", "C-S-F1" and
"C-A-S-F1". The order of the modifiers are not significant so "C-A-S-F1"
== "S-A-C-F1".
Any comments? Please advise.

>> Shift flag isn't valid with alphanumeric keys.
> Ah, ok. Am I right that it's enough just not to set the SHIFT mask in
> case isalnum() is true?
Right now I don't fully understand the way GRUB key handling works in
detail regarding modifiers.
Form what I've learned until now it seems to be like this:
    - all alphanumeric keys never have the SHIFT modifier set
    - all keys having a #define GRUB_TERM_KEY_* all modifiers (CTRL,
ALT, SHIFT) are possible
    - I'm not sure about GRUB_TER_BACKSPACE, GRUB_TERM_TAB and
GRUB_TERM_ESC. What modifiers are allowed for them?

BTW about using isalnum(): couldn't find any GRUB replacement. Is it
save to include <ctype.h> and use those functions?