On Thu, Sep 15, 2016 at 6:19 AM, Nick Østergaard <[email protected]> wrote:
> 2016-09-14 14:09 GMT+02:00 Oliver Walters <[email protected] > >: > > Hi all, > > > > First time submitting a patch, so here goes.... > > > > Welcome. Please be aware that your patches contain tabs, as per the > coding style policy; the indentation level for the KiCad source code > is defined as four spaces. Please do not use tabs. Ok, will fix. > > > The attached patch deals with a number of issues with regards to the > > footprint wizards manager. It started off as what I imagined was a fairly > > simple task to improve the UX of the FP wizards interface, but it evolved > > into something a bit more complex as I delved deeper into the source! > > > > Lovely to see some improments. I think there are great improvements in > here and I have tried to add some more comments inline. > > > Improvements are as follows: > > > > 1. I have done away with the use of a leading asterisk to designate the > > "units" of a wizard parameter. Multiple parameter types can now be > defined > > (integer, float, mm, mils, bool, etc..) > > > > 2. Input validation. Each type of parameter is now validated properly > within > > the wizards screen. Integer parameters can only be set to integers, > > dimensions can only be floating point, etc. > > > > 3. Boolean values are now treated properly. Simply click the cell to > toggle, > > rather than awkwardly typing "True" or "False" > > > > I this this works a bit strange right now because it switches between > showing the True/False text, but when clicking it will show the > checkbox. Why this switching between content? > > To make it easer than manually typing true or false as before I would > have expected either a combobox or a checkbox that is constantly > there. It looks like using a wxGridCellBoolRenderer will fix this. I'll look into this. > > 4. Multiple choice options available - If the python script specifies a > list > > of options, then a drop-down box will be displayed for that parameter > > > > Is there an example script that uses lists? I could not find any of > the default wizards proviing this option. I'll add an example script. > > > 5. Param designators. Instead of the row numbers being shown, each param > can > > optionally be assigned a designator (such as 'e' for pitch) which will be > > show to the left of that row. > > > > I think this makes sense. > > It would be cool if we in the future could render dimension arrows on > the plot for some things using those references. > That is a very cool idea. I had been planning on implementing the GetImage() method which would require a dimensional drawing for each wizard. However, with the ability to draw on the screen, you could easily show the dimensions only for the current page of parameters, which would de-clutter it a lot. I'm not sure how much work this would be, though. > > > 6. "Reset to default" - A new button in the top toolbar which resets all > > wizard parameters to their default values > > > > > > 7. More logical parameter checking within the python wizard helpers. > > Currently each parameter needs to be explicitly checked e.g. "CheckInt" > to > > make sure the value is a valid integer. This patch defines a Parameter > > holder class that automatically checks values based on their specified > type. > > Additionally, parameters can have other checks specified when they are > > added, e.g: > > > > AddParam("Pads","width",uMM, 2.5, min_value=0.1, max_value=5.5) > > > > 8. Fixed script import errors in the case of "bad" scripts. Currently if > a > > wizard contains any errors, the LoadPlugins functions fail and no > subsequent > > wizards are loaded. A simple try-except block has been added around the > > specific file loading so that one bad plugin file does not break the > others > > > > 9. Auto-sizing of the parameter grid. Not a huge deal but the grid width > now > > expands to fill the available space. > > > > General Notes: > > > > a) To provide the functionality for multiple unit types I have had to > > slightly break compatibility with the way that parameter types were > defined > > in the current wizards. However, I have designed the new Parameter class > (in > > kicadplugins.i) to be as close-to-compatible as it can be. > > > > Maybe some notes are required on how to update the older scripts to > ease a transition. > > > I have updated each of the default plugins to be compatible with the new > > system. Only minor changes were required. > > > > b) I have also consolidated the helper classes > > (HelpfulFootprintWizardPlugin) into the simpler FootprintWizard base > class. > > > > c) There is now a GitHub repository for FootprintWizards - > > https://github.com/KiCad/Footprint_Wizards - should this patch be > accepted I > > propose that the default wizards be further improved, and removed from > the > > source files. Instead, provide a link to the GitHub page or a > > download-helper for the scripts. This way the community can contribute > > quality wizards, and I shall endeavour to add some good documentation to > the > > wiki page for wizard creation. > > > > .diff is attached - hopefully this is the right way of doing this? > > > > Patches that can be applied with git am is good. In this case it seems > like all newlines in commit comments are stripped which is not so > good. > Any suggestions on how I could fix this? Should I be submitting one patch per commit, or a single patch as I have done here? Also, can I submit a pull-request straight from git or is the patch-email method preferred? > > I wonder why you make a construct like this in 0001. > #if defined( __MINGW32__ ) > path = A > #elif defined( __WXMAC__ ) > path = B > #else > path = A > #endif > > Why not like this? > #if defined( __WXMAC__ ) > path = B > #else > path = A > #endif I can simplify this as above. > > > Please let me know what I can do to help this process along. > > > > Regards, > > > > Oliver > > > > > > _______________________________________________ > > Mailing list: https://launchpad.net/~kicad-developers > > Post to : [email protected] > > Unsubscribe : https://launchpad.net/~kicad-developers > > More help : https://help.launchpad.net/ListHelp > > >
_______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
