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@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp > _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp