← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Run time advanced options

 

Hi Wayne,

Absolutely. That's one of the reasons to keep them in a separate file and make them read only with no UI. 

Most code here should really have a plan to eventually be removed. Eg in this case, the SVG one will be obsolete after it's deemed ready for general use, and the GTK3 one is only useful until legacy is removed.

I could see them being useful to allow things, like the SVG import filter, to be merged for developer or advanced tester access and get a bit more engagement than you'd get if you require a whole rebuild. For example, new plugins that need a bit of user hammer to check for corner cases. Making it runtime at least ensures it compiles and allows to add unit tests to the library code. Making an advanced config means we can get this engagement but without the implicit seal of approval that adding an out-of-the-box feature brings. 

Anything that end up being there long-term due to utility should be considered for a proper config with UI.

Cheers,

John

On 17 December 2018 18:13:43 GMT, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>Hi John,
>
>That fixed it thanks.  I do like this patch set but I am concerned that
>this could suffer from bit rot once these features are no longer run
>time options.  As long as it's kept up to date, I'm fine with merging
>it.  Anyone else have any objections or concerns?
>
>Cheers,
>
>Wayne
>
>On 12/17/2018 12:47 PM, John Beard wrote:
>> Hi Wayne,
>> 
>> Booleans are set as "0" or "1" by the wxConfigBase interface. I don't
>> think it translates "true" or "false" on read, so they'd probably be
>> defaults.
>> 
>> But I see I have somehow messed up the rebasing to make the SVG stuff
>> a separate commit and lost a line as well. Please try the attached
>> patches (1 and 3 should be the same) and a file with:
>> 
>> EnableLegacyCanvasWithGtk3 = 1
>> EnableSvgImport = 1
>> 
>> Sorry about that!
>> 
>> Cheers,
>> 
>> John
>> 
>> On Mon, Dec 17, 2018 at 5:13 PM Wayne Stambaugh
><stambaughw@xxxxxxxxx> wrote:
>>>
>>> Hey John,
>>>
>>> I'm not sure this is working correctly at least on windows.  I
>applied
>>> all 3 patches and created a kicad_advanced file (attached) but I'm
>not
>>> getting the debugging output I expect:
>>>
>>> [1588] (KICAD_ADVANCED_CONFIG) Init advanced config
>>> [1588] (KICAD_ADVANCED_CONFIG) Loading advanced config from:
>>> C:\Users\wstambaugh\AppData\Roaming\kicad\kicad_advanced
>>> [1588] (KICAD_ADVANCED_CONFIG) AllowLegacyCanvasInGtk3: false
>>>
>>> I don't see the EnableSvgImport option and the
>AllowLegacyCanvasInGtk3
>>> option is being read as false when it's set to true in the
>>> kicad_advanced file.  Am I doing something wrong?
>>>
>>> Cheers,
>>>
>>> Wayne
>>>
>>> On 12/13/2018 8:30 AM, John Beard wrote:
>>>> Whoops: the 16kB 0001 patch should not be there, that's a very old
>draft!
>>>>
>>>> Sorry,
>>>>
>>>> John
>>>> On Thu, Dec 13, 2018 at 12:57 PM John Beard
><john.j.beard@xxxxxxxxx> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> This is a patch for run time options, which are a more flexible
>alternative to compiler flags. Advantages include:
>>>>>
>>>>> * you can change the config without rebuilding
>>>>> * it's sensitive to XDG_CONFIG_DIR, so you can quickly flip back
>and forth
>>>>> * there's better documention for the config (it's in doxygen all
>in one place) and theres a trace to show it
>>>>> * better type safety if the config has a value
>>>>> * better compiler coverage, so less opportunity to break a
>different build which uses a different preprocessor block and better
>static analysis coverage
>>>>>
>>>>> These configs are not intended for general use by users so they
>are in their own file. You don't need this file, if you don't have it,
>you get defaults.
>>>>>
>>>>> The second patch uses the framework to add a config for the SVG
>import. The disablement is recast so it happens in a single place, and
>the same system could be used in future for other experimental
>importers. The reason it's done as a blacklist is so that a unit test
>could be written that *doesn't* disable the SVG plugin. However,
>nothing in Pcbnew can be unit tested yet, as I haven't worked out how
>to link Pcbnew code as a unit test.
>>>>>
>>>>> The third patch does another one for disabling legacy canvas on
>GTK3. Again, this is done at run-time to avoid conditionally compiling
>code.
>>>>>
>>>>> Cheers,
>>>>>
>>>>> John
>>>>
>>>> _______________________________________________
>>>> 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
>>>>
>>> _______________________________________________
>>> 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