kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxxxx>]

On 9/3/2011 12:10 PM, jean-pierre charras wrote:
> Le 03/09/2011 16:53, fabrizio a écrit :
>> jean-pierre,
>> I kind of get point. My consideration was more about a good looking
>> menu then 10% coherence. See in attachment how the great TextMate
>> does. I just think that the<>  is redundant because when for instance
>> "B" appears next to Bus in the Place menu, that means that the "B" is
>> the shortcut key for the command "place bus", it is not necessary to
>> indicate<B>. I also think that it is important to justify all keys
>> right. Notice that (in my opinion) the same thing should be done in
>> the ohtkeys list window.
>>
>> Now if commands change (or become inactive) because the mouse is
>> somewhere outside the sheet or whatever and the key does not work (as
>> I think is the case to mention), well no worries, I guess it is just
>> the way it is. Regardless, from an aesthetic point of view, I think we
>> should follow the TextMate way (which is also your way for CTRL-F for
>> Find).
>>
>> I do not think that people will ever understand the difference between
>> your CTRL-F and your<B>.
>>
>> This is just my opinion of course. I hope this will help.
>> cheers
>> Fabrizio
> 

Fabrizio,

I made the same mistake when I started messing around with the hot key
code.  The problem is one of semantics.  Hot keys are not accelerators
in the traditional way they are use in wxWidgets.  In other words
pressing the W key to begin drawing a wire is not the same command event
when selecting draw wire from the menu bar or tool bar.  From wxWidgets
point of view, pressing W does not generate the command event for
placing a wire as it would if you set the draw wire menu text to _(
"Wire\tW" ).  This would perform the same action as selecting wire from
the draw menu not the current behavior.  The other problem is that you
cannot use alphabetic characters without modifiers (Ctrl, Alt, etc.) in
a wxAcceloratorTable on Linux.  However, this does work on Windows.  At
least that used to be the case the last time I tried.  This would be a
much cleaner solution.  You cloud create a separate command ID and have
single event handler for both command IDs to handle both behaviors which
would eliminate a lot of redundant code.

> I agree the right justified hot key is by far better than <B>.
> But the difference is not for users, it is for wxWidgets event handler.

I think <B> is confusing in this case.  Pressing the B key does not
behave the same as selecting bus from the draw menu or tool bar.  Other
accelerators such as Ctrl+S (save) always end up in the same event
handler as selecting File->Save from the menu bar.

> 
> For instance, using the place bus command:
> When you click on the place bus menu (or the place bus tool on right
> side on Eeschema),
> only the place bus tool is selected.
> When you click on B, the place bus tool is selected (if not already
> selected) and a bus is always started from the mouse cursor location.
> Same behavior for place junction and other items.
> 
> The callback functions called by a key event is not the same callback
> functions called by clicking on a menu.
> 
> Unfortunately, when you create a menu with a hot key (i.e. a menu with a
> key right justified by a tab),
> when clicking on the hot key,
> the callback function called (by wxWidgets) by clicking on the key is
> always the menu callback function,
> not the keyboard callback function.
> Therefore if B is right justified (i.e. not displayed as <B>),
> place bus tool is selected (as it does) but a bus is not started,
> because the right callback function (keyboard function) is not called
> (this is the callback function relative to the menu that is called).
> (or the hotkey J does not place a junction ...)
> 
> Unfortunately, until now, I was not able to solve safely this problem.
> (Perhaps we can make the difference in menus callback functions by
> testing the mouse buttons state,
>  and expect no mouse button down when the keyboard is used, although it
> is not a safe way,
>  and this way needs changes in Kicad code)
> 
> If you know the right way (in wxWidgets) to know inside a callback
> function if it is called by the keyboard or by click on a menu,
> the <B> notation will be safely removed ( and the Kicad internal code
> simplified).

If the wxAcceleratorTable solution still does not work on Linux, a good
solution would be to create a custom command event class for handling
hot key events.  Another solution would be to always translate a hot key
into a command event which is more wxWidgets friendly than the current
design of the a large switch statement to handle each key code.

Wayne

> 
> 
>>
>>
>> On Sat, Sep 3, 2011 at 3:53 PM, jean-pierre charras
>> <jp.charras@xxxxxxxxxx>  wrote:
>>> Le 03/09/2011 15:29, fabrizio a écrit :
>>>>
>>>> Dear All,
>>>> this is my first patch, I hope it will be all right. This is what I
>>>> have
>>>> done.
>>>
>>> ....
>>>>
>>>> 2) hotkeys in the "Place" top menu are now properly displayed. They
>>>> are right justified and appear without the unnecessary "<>".
>>>
>>> This is not possible:
>>> As clearly written in comments in many places,
>>> when a key appear inside "<>" this is a comment, not a hotkey
>>> equivalent to
>>> the menu.
>>> This is due to the fact some hotkeys do not the same thing as the
>>> place menu
>>> items.
>>> Therefore we cannot use always \t instead of "<>"
>>> (It happen each time the command use the mouse position to do
>>> something, and
>>> this is the case in place menu).
>>> Unless someone knows, with wxWidgets,
>>> when the same command event is generated by a hot key or by clicking
>>> on the
>>> menu,
>>> how we know the command event is due to a click on the hot key or
>>> click on
>>> the menu.
>>>
>>>
>>> -- 
>>> 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
>>>
> 
>