← Back to team overview

kicad-developers team mailing list archive

Re: common/tool/tool_dispatcher needs fixing.

 

Hi JP,

This code definitely seems to have evolved a bit over time.  ProcessEvent() specifically returns true only if it was a hotkey that was handled, and not for anything else.  But I can’t find any code that benefits from that, so I suspect it’s vestigial.

I think the best thing to do is to make it uniform (with respect to the Mac) and meet expectations (regarding the handled flag), and then fix the fallout from whatever it was that we didn’t understand about how it was supposed to work.

I’ll play with it a bit before committing to make sure nothing’s /really/ broken.

Cheers,
Jeff.


> On 9 Jul 2019, at 19:09, jp charras <jp.charras@xxxxxxxxxx> wrote:
> 
> Hi Jeff,
> 
> Sorry to bother you, but could you have a look into this file, and
> especially into void TOOL_DISPATCHER::DispatchWxEvent( wxEvent& aEvent ).
> 
> There are 2 things that need fixing  (related to key event handling),
> because our key event handler has bugs:
> 
> 1 - the first is related to ESC key:
> if a ESC key is typed, first avent is set:
> evt = TOOL_EVENT( TC_COMMAND, TA_CANCEL_TOOL );
> and later:
> m_toolMgr->ProcessEvent( *evt ); returns false: this is a bug.
> Obviously, the key event is handled, and this line should return true.
> (I recently added a workaround, but fixing the bug is better)
> A golden rule on wxWidgets is: skip the key event if not handled by a
> window or frame or..., or do not skip it when handled.
> 
> 2 - the second issue is OSX specific:
> Have a look to line 492 to 495:
> the char event is not skipped ( when not handled ) on OSX.
> I am thinking this is incorrect (at least in Debug mode it should be
> skipped when not handled).
> I am guessing this ugly hack comes from the time the tool dispatcher was
> broken, and skip any key event, even when it was handled (and that was a
> serious bug).
> This is fortunately no longer the case.
> But this conditional compilation creates a difference between OSX and
> other OS.
> We don't need that.
> And it explains why you did not see some bugs related to hotkeys.
> 
> Thanks.
> 
> -- 
> Jean-Pierre CHARRAS
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp



Follow ups

References