← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Saving plot dialog settings


> 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 little bit here is goofy though, for a function comment:

+    /**
+       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.


Follow ups