Allowing shortcuts without the B_COMMAND_KEY modifier

[leavengood]

As ​discussed on the mailing list I wanted to add the ability for there to be menu shortcuts which do not use any modifiers. I also wanted the ability to have shortcuts for the function keys (F1-F12.)

I have written the code to do so and it seems to work well. I'd still like to test an app written against this code with the libbe.so that does not have this code to ensure there aren't issues there (I think it should work OK.) And obviously existing apps should be tested thoroughly to ensure there is not breakage.

But in the meantime I'll attach a patch here for review.

[leavengood]

First revision of patch to allow for shortcuts without the command key

[leavengood]

Take ownership.

[leavengood]

One possible problem with this patch is the removal of a check for the B_COMMAND_KEY before checking shortcuts for a match. This means keyboard handling could be slower since every keypress would need to check through all the shortcuts. I may look for optimizations such as only doing this if there exist shortcuts which don't use modifiers. But that may be a premature optimization without benchmarking.

[axeld]

I think shortcuts in menu without a modifier should stay an exception. I wouldn't even mind to only have that for the function keys per style guide.

I have a few comments, though:

• B_NO_MODIFIER does not belong to InterfaceDefs.h. It should be a special flag for use with AddShortcut(); it does not have any meaning outside of this call. In any case, I would go for a larger value, not just the next available spot.
• Constraining the allowed keys to the F-keys would considerably ease the check whether or not a shortcut key had been pressed. Besides that, I don't think the speed is really an issue here. You shouldn't be able to let a computer sweat with typing alone these days :)
• I'm not sure if a check for "modifiers == 0" will work as expected. IIRC things like the num lock or such things may be part in the modifiers as well (or are those masked out before?).
• "return else return" is bad style and should be avoided; the "else" is superfluous and confusing.
• In general, I'm not a friend of using the F-keys for shortcuts, as they are pretty much meaningless keys, and far away from your hands. Their use should be restricted somehow via a style guide.

[leavengood]

Replying to axeld:

I think shortcuts in menu without a modifier should stay an exception. I wouldn't even mind to only have that for the function keys per style guide.

Well my main motivation for this was the fact that ShowImage and WebPositive and probably a lot of apps already have "shortcuts" that are single keys, except that they are hidden in the app's keyboard handling code and either need to be documented or discovered by accident (or they happen to match shortcuts from other operating systems.) I think pretty much any application which is not strictly a text editor will have these anyhow. So the idea of having them as menu shortcuts is that they are then easily discoverable by the user.

Examples in ShowImage are 1 and 0 for normal size and fit to window, + and - for zooming, [ and ] for rotating the image, spacebar for starting and stopping the slideshow, the arrow keys for navigation, etc. Right now a lot of these either have pointless aliases as shortcuts (like Alt-1 and Alt-0) or no shortcut at all. I think it would be better to just have the single keys as the shortcuts and be done with it (then we also don't need the special keyboard handling code.)

I have a few comments, though:

• B_NO_MODIFIER does not belong to InterfaceDefs.h. It should be a special flag for use with AddShortcut(); it does not have any meaning outside of this call. In any case, I would go for a larger value, not just the next available spot.

Yeah I think that is fine. I just put it there but it definitely doesn't fit. But obviously it needs to still be a bit-flag which matches the style of the modifiers (so as not to be confused with them.) I could make it 0x80000000.

• Constraining the allowed keys to the F-keys would considerably ease the check whether or not a shortcut key had been pressed. Besides that, I don't think the speed is really an issue here. You shouldn't be able to let a computer sweat with typing alone these days :)

I guess that is a good point. I just added some printf debugging and it seems like a lot of work for each key. But it still is fast even with all the printf output so I suppose it is fine.

• I'm not sure if a check for "modifiers == 0" will work as expected. IIRC things like the num lock or such things may be part in the modifiers as well (or are those masked out before?).

The modifiers are masked before that but I learned the above the hard way when I commented out the part that masks it :)

"Why is byte 1 always 2! Oh, NUM LOCK is on!"

• "return else return" is bad style and should be avoided; the "else" is superfluous and confusing.

Good point. I sort of figured that out today too when adding the AutoAdjustingNavigator to ShowImage.

• In general, I'm not a friend of using the F-keys for shortcuts, as they are pretty much meaningless keys, and far away from your hands. Their use should be restricted somehow via a style guide.

Yeah maybe. I just happened to add those too since I know some people might want to use them. But if they will be frowned upon in the style guide we could just not support them. But for better or worse there are a lot of "standard" uses for them from other operating systems.

[pulkomandy]

Moving out of Beta1: new feature, not in BeOS R5.

[pulkomandy]

Patch migrated to Gerrit: ​https://review.haiku-os.org/c/haiku/+/2603

[Manuel]

Please, include support for the 'Pause' key.