Re: [PATCH] Detect key modifier status in 'sleep --interruptible'

[Colin Watson]

On Tue, Aug 25, 2009 at 12:41:20AM +0200, Robert Millan wrote:
> On Mon, Aug 24, 2009 at 11:04:19PM +0100, Colin Watson wrote:
> > +/* See http://heim.ifi.uio.no/~stanisls/helppc/bios_data_area.html for a
> > +   description of the BIOS Data Area layout.  */
> > +struct grub_machine_bios_data_area
> > +{
> > +  grub_uint8_t unused1[0x17];
> > +  grub_uint8_t keyboard_flag_lower; /* 0x17 */ 
> > +  grub_uint8_t keyboard_flag_upper; /* 0x17 */ 
> > +  grub_uint8_t unused2[0xf0 - 0x19];
> > +};
> 
> Why split the keyboard field in upper/lower?  We have working 16-bit types :-)

We only need the lower one and counterintuitively (for a little-endian
system) I seemed to have to look at bits 8-11 of the 16-bit type rather
than bits 0-3. Life was too short to worry about what was wrong and I
decided to only fetch the byte we needed. I suppose we could delete the
declaration of keyboard_flag_upper.

> > +static int
> > +grub_check_keyboard (void)
> > +{
> > +  int mods = grub_getkeystatus ();
> > +  if (mods >= 0 && (mods & GRUB_TERM_STATUS_SHIFT) != 0)
> > +    return 1;
> > +
> > +  if (grub_checkkey () >= 0 &&
> > +      GRUB_TERM_ASCII_CHAR (grub_getkey ()) == GRUB_TERM_ESC)
> > +    return 1;
> > +
> > +  return 0;
> > +}
> 
> I'm not sure if hardcoding ESC and SHIFT here makes this a bit too ad-hoc.

Maybe not much more than the previous hardcoding of Escape alone?

> IIRC you had a more generic approach in mind?

We talked on IRC about adding a magic "keystatus" environment variable
that user code could inspect. I'm not sure whether this will actually
make things any simpler at the moment, though - I don't think there's a
way to do bitwise operations in the shell-like scripting interface (you
can use 'test' to test greater/less than, but nothing like "&"). Would
three magic variables, keystatus_shift, keystatus_ctrl, and
keystatus_alt, be too inelegant?

I'm open to other ideas. 'sleep --interruptible 0' is not an entirely
natural way to express what I'm looking for, so I'm not particularly
attached to it. 'if sleep --interruptible 0; then set timeout=0; fi' is
suboptimal because it means that if either (a) core.img is too old to
have getkeystatus support or (b) the current console driver does not
support getkeystatus then the menu will be skipped without a way to
access it at run-time. I'd much rather have something that allowed
zero-timeout-with-Shift-to-access-menu if and only if we have a new
enough core.img and the console driver has getkeystatus support, and
otherwise degraded to something else sensible.

-- 
Colin Watson                                       address@hidden