← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Saving plot dialog settings

 

On 1/4/2011 10:31 PM, Dick Hollenbeck wrote:
> 
>> The previous patch has a bug. It incorrectly complains about no layers
>> being selected. This patch works better.
>>
>> marco
>>
>> On Sun, Jan 2, 2011 at 11:40 PM, Marco Mattila <marcom99@xxxxxxxxx> wrote:
>>> Hi,
>>>
>>> During previous discussions about subtracting masks from silkscreen
>>> layers when plotting gerbers, Dick mentioned that saving plot settings
>>> is not quite up to date. Currently some plot settings to be
>>> saved/loaded are defined in pcbnew_config.cpp. However, that requires
>>> that corresponding variables in the PCB_PLOT_PARAMS class are public.
>>> My proposal is to let the class itself take care of saving/loading the
>>> settings. That's mainly what the attached patch changes. In addition,
>>> all plot dialog settings should now be included. The wxConfig key
>>> names are also edited to start with "Plot", and layer selections are
>>> combined into a single bit mask. Moreover, some global variables in
>>> pcbplot.cpp have been moved into the class, too. g_PcbPlotOptions
>>> itself is still global. However, I think that it could be moved to be
>>> a member of the WinEDA_BasePcbFrame. That way it should be accessible
>>> where needed without being global. And if necessary, plot settings
>>> could still be loaded only once when pcbnew starts (now they are
>>> loaded every time the plot dialog constructor is called). In addition
>>> drill file generation settings could be included into PCB_PLOT_PARAMS.
>>> I can continue working on this if my approach sounds reasonable.
>>>
>>> marco
> 
> I would say the patch looks like an improvement worth committing.
> 
> Thanks.  If anyone has concerns they should voice them.

This is a nice patch so I'm all for committing it.  I would have liked it even
more if there was a way to avoid using a the global variable g_PcbPlotOptions.
 Thanks Marco for your help.

Wayne

> 
> This little bit here is goofy though, for a function comment:
> 
>      PCB_PLOT_PARAMS();
> +    /**
> +       Function LoadSettings
> +       loads plot settings from wxConfig system.
> +       @param aConfig is a pointer to a wxConfig.
> +     */
> +    void        LoadSettings( wxConfig* aConfig );
> +    /**
> +       Function SaveSettings
> +       saves current plot settings to wxConfig system.
> +       @param aConfig is a pointer to a wxConfig.
> +     */
> +    void 
> 
> In the future, please leave a space between these like the coding standard
> says, and fill in the vertical stars.
> 
> My vote is to commit.
> 
> Dick
> 
> 
> _______________________________________________
> 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
> 



Follow ups

References