Re: keyboard: ISO_Left_Tab

[Andreas Mohr]

Hi,

On Sat, Nov 06, 2004 at 07:30:20AM -0800, Walt Ogburn wrote:
> - /* Special case: X turns shift-tab into ISO_Left_Tab. */
> - /* Here we change it back. */
> - if ((ret == 0) && (keysym == XK_ISO_Left_Tab))
> - {
> -
>    ret = 1;
> -
>   lpChar[0] = 0x09;
> - }
> - if (ret == 0)
>       {
>       BYTE dead_char;

Two comments:
a) A return value of 0 probably is much more common than the XK_ISO_Left_Tab
keysym, so the XK_ISO_Left_Tab check should be first to not have to check both
conditions in the event that ret is 0.
(the compiler might do reordering according to check probability, though,
but OTOH I doubt that it knows which one is more likely)
b) there's another if (ret == 0) *right below*, so why not add the
XK_ISO_Left_Tab check there? Again useless waste of resources...

Thanks for this patch!

Andreas Mohr

keyboard: ISO_Left_Tab

[Walt Ogburn]

Hi Andreas,

On (a), you are certainly correct.  I'll swap the order and resubmit.

As for (b), the "if (ret == 0)" just below is followed by an "else" which
does the actual unicode conversion.  I wanted to put in the tab character
and change ret to 1 in time to get into the "else".  It the ISO_Left_Tab
check went under the existing "if (ret == 0)", it would have to have its
own MultiByteToWideChar call, or use a goto.  This way seemed a lot more
readable.

Thanks,
Walter

On Sat, 6 Nov 2004, Andreas Mohr wrote:
> Hi,
>
> On Sat, Nov 06, 2004 at 07:30:20AM -0800, Walt Ogburn wrote:
> > - /* Special case: X turns shift-tab into ISO_Left_Tab. */
> > - /* Here we change it back. */
> > - if ((ret == 0) && (keysym == XK_ISO_Left_Tab))
> > - {
> > -
> >    ret = 1;
> > -
> >   lpChar[0] = 0x09;
> > - }
> > - if (ret == 0)
> >       {
> >       BYTE dead_char;
>
> Two comments:
> a) A return value of 0 probably is much more common than the XK_ISO_Left_Tab
> keysym, so the XK_ISO_Left_Tab check should be first to not have to check both
> conditions in the event that ret is 0.
> (the compiler might do reordering according to check probability, though,
> but OTOH I doubt that it knows which one is more likely)
> b) there's another if (ret == 0) *right below*, so why not add the
> XK_ISO_Left_Tab check there? Again useless waste of resources...
>
> Thanks for this patch!
>
> Andreas Mohr