kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

On 2/28/2018 3:46 PM, Wayne Stambaugh wrote:
> On 2/28/2018 2:31 PM, jp charras wrote:
>> Le 28/02/2018 à 17:45, Wayne Stambaugh a écrit :
>>> In the process of attempting to fix this bug[1], I ran into an issue
>>> with the '/' hotkey for all main frames.  For some reason, the hotkey
>>> help dialog is being displayed for both '/' and '?' keys which broke the
>>> track posture switching in pcbnew.  This bug only seems to affect
>>> windows.  The attached patch fixes the issue but changes the menu entry
>>> shortcut text from '?' to 'shift+?' which would make pcbnew different
>>> from all of the other mainframe windows which is ugly but it's better
>>> than a broken hotkey.
>>>
>>> While I was at it, I took a look at the schematic editor and sure enough
>>> the same behavior exists except that the same change as the attached
>>> patch does not fix the issue so the bus wire entry hotkey is broken.
>>> When I set a debugger breakpoint in the EDA_DRAW_PANEL::OnKeyEvent()
>>> function, it is not triggered for either a '/' key.  The '?' key does
>>> trigger the breakpoint.  Did someone create a character hook somewhere
>>> that could be preventing EDA_DRAW_PANEL from ever receiving the '/' key
>>> presses and passing them directly to the SCH_EDIT_FRAME?  I cannot find
>>> any reason why the EDA_DRAW_PANEL::OnKeyEvent() event handler is not
>>> being called for this key on windows.
>>>
>>> Cheers,
>>>
>>> Wayne
>>>
>>> [1]: https://bugs.launchpad.net/kicad/+bug/1751812
>>
>> Wayne,
>> Because the hotkeys can be redefined by user, I don't think your fix is working.
> 
> '/' is the default hotkey for change track posture in pcbnew and place
> bus wire entry in eeschema.  So user hotkey definitions are not relevant
> since I have not changed any of my hotkey defaults.  What is odd is that
> this fix works for pcbnew but not eeschema.
> 
>>
>> Moreover it happen mainly in US keyboards
>> (I am guessing the chars '?' and '/' are the same keyboard key)
> 
> They are on the same key on US keyboard.  That being said '/' is not the
> same as key code as 'shift+/' or at least they shouldn't be the same.
> 
>>
>> On Windows, the key events are really tricky, and I never be able to fix the accelerators versus
>> hotkeys issues.
>>
>> In this case the issue is due to the complexity of keys events:
>> The simplified event list is:
>>
>> - first the EVT_CHAR_HOOK_EVENT is send with key code = '/'
>> if not captured:
>> - send EVT_CHAR_EVENT is send with key code = '?'
>>
>> and in fact there are more events sent, both using EVT_CHAR_HOOK_EVENT and EVT_CHAR_EVENT.
>>
>> This is true for each key: a event is send with code corresponding to the non shifted code, and then
>> to the shifted code (if the envent is not captured)
>>
> 
> While all of this is informative it still does not answer the question
> as to why the '/' character is never getting handled in
> EDA_DRAW_PANEL::OnKeyEvent() while the '?' character is being handled.
> I'll keep digging around because something is different between the
> schematic and board editors.
> 

JP,

I figured out what is going on with some simple tracing.  On windows
with US keyboard layouts, the '?' key is sent as a 'shift+/' which isn't
unexpected.  What is unexpected is that we are only allowing the shift
key to be passed when keys 'a' - 'z' and 'A' - 'Z' are pressed by this
code and associated comment:

    /* Disallow shift for keys that have two keycodes on them (e.g.
number and
     * punctuation keys) leaving only the "letter keys" of A-Z.
     * Then, you can have, e.g. Ctrl-5 and Ctrl-% (GB layout)
     * and Ctrl-( and Ctrl-5 (FR layout).
     * Otherwise, you'd have to have to say Ctrl-Shift-5 on a FR layout
     */
    bool keyIsLetter = ( localkey >= 'A' && localkey <= 'Z' ) ||
                       ( localkey >= 'a' && localkey <= 'z' );

    if( event.ShiftDown() && ( keyIsLetter || localkey > 256 ) )
        localkey |= GR_KB_SHIFT;

This means any keys such as ';:', ''"', ',<', '>.', '/?', etc. will most
likely be broken when used as hotkeys.  I could easily add the '/' to
the keyIsLetter text code but that is an ugly hack which will most
likely break for non-us keyboard layouts.  Any ideas how to implement
this cleanly or is this just an ugliness we have to live with due to
keyboard layout issues?

Cheers,

Wayne