kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxxxx>]

On 7/23/2010 11:01 AM, Jerry Jacobs wrote:
> On 21-07-10 15:21, Wayne Stambaugh wrote:
>> On 7/21/2010 3:52 AM, Jerry Jacobs wrote:
>>> Because almost all toolbar items are not in the menubar of the module
>>> editor I wrote a small patch.
>>> Maybe someone can take care of the few TODO's inside the patch which I
>>> was not able to get.
>>
>> Jerry,
>>
>> Most of your TODO's suggest creating new IDs for the module editor instead of
>> reusing the IDs from PCBNew.  Is there any technical reason to do this?  As
>> long as the ID is descriptive enough than I see no reason to create another ID
>> specifically for the module editor.  We already reuse wxID_CUT, wxID_COPY,
>> wxID_PASTE, etc.  I did some ID rationalization a while back that significantly
>> reduced the number of redundant IDs and unnecessary recompiling.  I would
>> rather avoid adding new IDs unless there is a good reason to do so.
> 
> Yeah I didn't know why there are mixed IDs but now I understand, and this a
> good choice. So the TODO's can be forgotten.

When I get some time I would like to do some additional ID clean up.  Creating
a common ID like ID_ADD_LINE used by all of the apps that draw a line is the
same as using wxID_OPEN for opening files.  I don't see how ID_PCB_ADD_LINE,
ID_MODEDIT_ADD_LINE, ID_EESCHEMA_ADD_LINE make the code any more readable and
just add to already large number of IDs Kicad uses.

> 
>>>
>>> The File ->  Close  needs a ID handler and need's to be fixed.
>>> And maybe some entries should not be selectable in some cases like
>>> undo,redo and the placement of components if there is no module
>>> loaded or new module created.
>>>
>>> Maybe all the *modedit* files can better be moved to a subdirectory if
>>> the code will be growing.
>>> And also moving the dialog_*  files. Because we get
>>> already a nice amount of files and directories are handy to sort things up.
>>
>> This is good idea for the eeschema subdirectory as well as it is getting rather
>> unwieldy.
>>
>>>
>>> See the attached patch, it applies to rev 2418 without problems.
>>
>> Your patch has some code formatting issues.  There are some indenting alignment
>> problems.
>>
>> +        item = new wxMenuItem( openSubmenu,
>> +                    ID_MODEDIT_IMPORT_PART,
>> +                    _( "from File" ),
>> +                    _( "Import a footprint module from a existing file" ) );
>>
>> should be
>>
>> +        item = new wxMenuItem( openSubmenu,
>> +                               ID_MODEDIT_IMPORT_PART,
>> +                               _( "from File" ),
>> +                               _( "Import a footprint module from a existing
>> file" ) );
>>
>> There is also some trailing white space as well.  If your editor supports
>> showing white space, you may want to turn it on.  Uncrustify will also remove
>> any trailing white space.
> 
> I use vim and have not all settings yet to manage the coding style for the
> kicad project as my coding style is a bit different. But I will have a look at
> uncrustify then, it installs on my Apple Mac so that is nice.

Uncrustify is not a run it and commit it operation.  You may still need to
perform some manual indenting.  I have found that uncrustify struggles when
indenting strings inside the internationalization macro.

> 
>> Thanks,
>> Wayne
> 
> Hope somebody will find some time to clean the patch and review it for pushing
> it upstream. I have write rights to the repository but don't commit major (UI)
> changes.

When I get a few minutes, I'll clean up this patch and commit it if no one else
objects.

Wayne

> 
> Kind regards,
> Jerry Jacobs
> 
> 
> _______________________________________________
> 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
>