kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

Patch committed in r6424.  Thanks.

On 1/5/2016 1:01 PM, Chris Pavlina wrote:
> Warnings are already fixed in an upcoming patch, go ahead and commit and 
> I'll send another patch to apply on top (it fixes a couple other minor 
> things too).
> 
> I'll have a look at the redundant pointer storage, that might be left 
> over from some old code. I'll fix that if I can as well.
> 
> On Tue, Jan 05, 2016 at 01:00:21PM -0500, Wayne Stambaugh wrote:
>> This patch looks fine.  I do like the tree view better than the tabbed
>> view.  I got two build warnings using GCC 5.3.0 which probably should be
>> fixed.  If it will prevent your third patch from applying cleanly, I can
>> commit it as is and you can fix them later.  Here are the build warnings:
>>
>> [ 41%] Building CXX object
>> common/CMakeFiles/common.dir/dialogs/dialog_hotkeys_editor_base.cpp.obj
>> In file included from C:/msys64/mingw32/include/wx-3.0/wx/defs.h:695:0,
>>                  from C:/msys64/mingw32/include/wx-3.0/wx/wx.h:14,
>>                  from
>> C:/msys64/home/wstambaugh/src/kicad/product/include/fctsys.h:28,
>>                  from
>> C:/msys64/home/wstambaugh/src/kicad/product/common/dialogs/dialog_hotkeys_editor.cpp:26:
>> C:/msys64/home/wstambaugh/src/kicad/product/common/dialogs/dialog_hotkeys_editor.cpp:
>> In member function 'void HOTKEY_LIST_CTRL::DeselectRow(int)':
>> C:/msys64/home/wstambaugh/src/kicad/product/common/dialogs/dialog_hotkeys_editor.cpp:63:33:
>> warning: comparison between signed and unsigned integer expressions
>> [-Wsign-compare]
>>      wxASSERT( aRow >= 0 && aRow < m_items.size() );
>>                                  ^
>> C:/msys64/home/wstambaugh/src/kicad/product/common/dialogs/dialog_hotkeys_editor.cpp:
>> In member function 'void HOTKEY_LIST_CTRL::OnChar(wxKeyEvent&)':
>> C:/msys64/home/wstambaugh/src/kicad/product/common/dialogs/dialog_hotkeys_editor.cpp:115:40:
>> warning: unused variable 'parent' [-Wunused-variable]
>>                  HOTKEYS_EDITOR_DIALOG* parent =
>> static_cast<HOTKEYS_EDITOR_DIALOG*>( m_parent );
>>
>> I also noticed that this dialog is storing the pointer to the parent
>> window in the m_parent member variable.  This is redundant since the
>> wxDialog base class saves the pointer to the parent window and can be
>> accessed using wxWindow::GetParent().  GetParent() is virtual in case
>> you need to overload it to return a specific type of parent window.
>> This seems to be a pattern in a lot of our dialogs which I would like to
>> see go away.
>>
>> Let me know if you want me to commit it as is or wait for a patch to fix
>> the build warnings.
>>
>> Thanks,
>>
>> Wayne
>>
>> On 1/4/2016 4:28 PM, Chris Pavlina wrote:
>>> Second step is to remove the pages from the hotkeys dialog and make it a 
>>> single-page control, so we don't have nested tabs when it's brought into 
>>> the already tabbed options dialog. This patch removes the pages, in 
>>> favor of categories in the wxTreeListCtrl. It also makes the final few 
>>> steps towards making the HOTKEY_LIST_CTRL fully embeddable.
>>>
>>> This patch applies on top of the previous one. I recommend keeping them 
>>> as separate commits, just in case I've created any bugs (makes them 
>>> easier to track down).
>>>
>>> There are a couple known GUI "quirks" - the columns are sized slightly 
>>> wrong when there is a vectical scrollbar, and the column sizing changes 
>>> a bit erratically when resizing the dialog. These are wx bugs. I'm going 
>>> to address them after the logical bits are complete.
>>>
>>> On Mon, Jan 04, 2016 at 03:05:33PM -0500, Chris Pavlina wrote:
>>>> Hi,
>>>>
>>>> First step in getting the hotkey configuration inside the main 
>>>> preferences dialog is to refactor it a bit. This patch:
>>>>
>>>> - Replaces the wxListCtrl with a wxTreeListCtrl, allowing expandable 
>>>>   categories in a future change.
>>>>
>>>> - Cleans up the code to make HOTKEY_LIST_CTRL function a bit better on 
>>>>   its own.
>>>>
>>>> - Migrates this dialog as well to TransferData{To,From}Window, use 
>>>>   matching TransferData{To,From}Control methods on HOTKEY_LIST_CTRL so 
>>>>   it is easy to embed.
>>>>
>>>> Despite being replaced by a tree-type control, none of the behavior has 
>>>> been changed yet. The appearance changes slightly due to wxTreeListCtrl 
>>>> looking a bit different.
>>>>
>>>> --
>>>> Chris
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> 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
>>
>> _______________________________________________
>> 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