← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH]

 

On 12/11/2011 10:10 PM, Guillaume Simard wrote:
> I have noticed I had not followed the indentation standards for this patch
> (using tabs and braces not at the right level).
> 
> Attached is a new version with this problem fixed, hopefully.
> 
> Guillaume
> 
> On Sat, Dec 10, 2011 at 11:56 PM, Guillaume Simard <gsimard@xxxxxxxxx
> <mailto:gsimard@xxxxxxxxx>> wrote:
> 
>     As per the TODO.txt file, there was a bug with CvPCB that would prevent an
>     already assigned footprint from being shown in the preview window when the
>     selected component changed :
> 
>     CvPCB
>     -----
>     * Preview of the already assigned footprint.
> 
> 
>     This is fixed with the attached patch.
> 
>     What was done:
> 
>     Whenever the selection in the components listbox is changed
>     (CVPCB_MAINFRAME::OnSelectComponent), the list of footprints is searched
>     for the actual footprint previously selected, and if found, that footprint
>     is selected in the listbox.
> 
>     The effect of this is that when the preview frame is opened, the selected
>     footprint is now shown. Because this was done in the OnSelectComponent
>     function, it also work when the next component is automatically selected
>     after double clicking a footprint for the current component.
> 
> 
>     To write the patch, I mostly used code as-is from CvPCB, so the notation
>     should respect the convention.
> 
>     As this is my first patch, I hope all of this has been done properly, and
>     any suggestions would be welcomed.
> 
>     Regards,
>     Guillaume Simard
> 
> 
> 
> 
> -- 
> Guillaume Simard, B.Ing., M.Sc.A.
> e.   gsimard@xxxxxxxxx <mailto:gsimard@xxxxxxxxx>
> t.    514-501-5707

Guillaume,

One minor comment on this patch.  Automatic member variables should start with
a lower case letter to indicate that the variable is local in nature.
Therefore this line in your patch:

+        wxString FootprintName;

should look like:

+        wxString footprintName;

I know that there is lots of legacy code in KiCad that violates this rule but
we are working on that.  It makes the task much easier to accomplish if we
don't have to correct new code.

Thanks,

Wayne


References