← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] - File format shim for clearance data

 

Hi Wayne,

Patch reverted.

However, I think your concern is misplaced.  If we want to prevent dataloss (ie: from reading a 6.0 file into 5.0), then we should warn the user based on the version string (and give them a chance to say “I still want to open”).

But either way, actually failing to read the file leaves the user in a pickle (especially when it’s easy enough for them to try out a nightly that may very well be ahead of their stable).

(And for that reason I think it’s important to put it into 5.0, as otherwise it won’t help until we start making 7.0 file format changes.)

Cheers,
Jeff.

> On 16 Mar 2018, at 18:00, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
> 
> Jeff,
> 
> Please revert this patch.  Any changes to the board file parser and/or
> formatter need to be discussed before the changes are committed.  It has
> been the intention that the board parser be pendantic by design to
> prevent data loss by ignoring unknown formatting.  Allowing anything
> outside of the expected formatting in the board file is not something
> that I want to introduce without some discussion.  Even should we decide
> to accept this patch, I would prefer we put it off until v6.
> 
> That being said, the patch fails to build on windows with following error:
> 
> C:/msys64/home/wstambaugh/src/kicad-trunk/pcbnew/pcb_parser.cpp: In
> member function 'void PCB_PARSER::parseUnknown()':
> C:/msys64/home/wstambaugh/src/kicad-trunk/pcbnew/pcb_parser.cpp:1269:12:
> error: request for member 'LogText' in '__acrt_iob_func(2)', which is of
> pointer type  FILE* {aka _iobuf*}' (maybe you meant to use '->' ?)
>     stderr.LogText( msg );
>            ^~~~~~~
> 
> Cheers,
> 
> Wayne
> 
> On 3/16/2018 1:08 PM, Jeff Young wrote:
>> Perhaps somewhat germane to this discussion I have removed the strict-format nags from the PCB parser.  It now logs warnings to stderr but otherwise ignores structures it doesn’t understand.
>> 
>> I’m not sure that helps hauptmech much as if the file is subsequently written the unknown markup will be lost, but I thought I’d mention it.
>> 
>> Cheers,
>> Jeff.
>> 
>>> On 7 Mar 2018, at 20:12, hauptmech <hauptmech@xxxxxxxxx> wrote:
>>> 
>>> Hi Thomasz,
>>> 
>>> I hope I'm able to address you concerns. I'm stuck using kicad stable in many situations. I brought clearances up for discussion last July but no one moved on it, nor are they required to. Clearance management is a major pain point for me so I wrote a fix. This patch will let us (me and the people I collaborate with) work using version 5, but open and close files written with a version patched with clearance handling code.
>>> 
>>> I believe that code exactly like this will go into version 6. Getting it in earlier makes a huge difference to me, so I'm submitting it.
>>> 
>>> On 07/03/18 23:30, Tomasz Wlostowski wrote:
>>>> Hi hauptmech,
>>>> 
>>>> I'm sorry but IMHO we can't accept your patch:
>>>> - it changes the file format while we are already in feature freeze.
>>>> This is a way too big change to accept for the V5.
>>> 
>>> This patch simply adds tokens to the file format. No clearance data is saved for files that use the netclass only. Files without clearance tokens continue to remain without them.
>>> 
>>> Yes it is a backward compatible file format change, but it does no harm to V5 files already in the wild.
>>> 
>>>> - we are planning to overhaul the clearance/design rules system in V6.
>>>> Storing the clearance *DIRECTLY* for each track segment/via will
>>>> conflict with any more sophisticated design rule management system.
>>>> 
>>> I'm glad you are planning this. I am sure that regardless of the sophistication of the rule system, you will store clearance directly for exactly the same reason that track width is stored directly now. There are always exceptions to the rules.
>>> 
>>> If kicad choose a direction that does not store clearances per item, then it is easy to rip these few lines back out.
>>> 
>>> Did this answer your existing concerns about this patch? Are there any other concerns you have about this patch?
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> 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
>> 
> 
> _______________________________________________
> 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