@Jeff, I still have to test the round tripping to make sure there is no data loss for potential combinations of unknown tokens. As soon as I have tested it, I will let you know.
On 3/27/2018 1:34 PM, Jeff Young wrote: > 5.0, 6.0 or abandon? > >> On 20 Mar 2018, at 16:47, Jeff Young <[email protected] >> <mailto:[email protected]>> wrote: >> >> Hi Wayne, >> >> This solution /*is*/ pretty much “property” tokens except that: >> >> 1) it’s more flexible (properties can be arbitrary s-expressions) >> 2) it handles 3rd-party-needs and our own with a single infrastructure >> >> I was going to add (3) it requires name-spacing, but “property” tokens >> will require name spacing anyway (to keep 2 different 3rd parties from >> colliding). The only difference is that we get the default name-space >> to ourselves. >> >> In fact, I think the “property” angle is a much better angle to look >> at it from. Rather than think of it as a file-format issue, think of >> it as s-expression meta-data for BOARD_ITEMS. If we later decide that >> "(hole-to-hole-clearance 0.25)" should define a distance used by DRC, >> then great. Until then, it’s just an opaque s-expression. >> >> I also think it demonstrates that the risk of wiping out stuff editing >> a future board with an older version isn’t really there. Just like >> “properties”, we’ll round-trip whatever meta-data was in the board. >> Sure, new BOARD_ITEMS that you add won’t have the future data, but >> why would they? >> >> (And if we remain concerned José’s force-save-as solution is an >> excellent idea.) >> >> The code complexity it adds to the parser is tiny. It turned out to >> be far easier to implement than anticipated. >> >> Cheers, >> Jeff. >> >> >>> On 20 Mar 2018, at 14:48, Wayne Stambaugh <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> Jeff, >>> >>> I wanted some time to think about this. It has been discussed before. >>> While I'm not completely opposed to it, I still haven't found a more >>> compelling argument to convince me that it is a better idea than using >>> strict parsing. As a user, it does have a certain appeal. As a >>> developer, it opens a Pandora's box of issues that once they are in >>> play, could be extremely difficult to reverse. Please see my responses >>> below. >>> >>> On 3/20/2018 9:40 AM, Jeff Young wrote: >>>> @Wayne, did you have any thoughts on this iteration? >>>> >>>>> On 20 Mar 2018, at 10:22, Jeff Young <[email protected] >>>>> <mailto:[email protected]> >>>>> <mailto:[email protected]>> wrote: >>>>> >>>>> Oh, and I don’t think this materially alters the support equation: we >>>>> already have to deal with hand-edited boards, so what’s in the file is >>>>> never guaranteed to be something Kicad put there. >>> >>> True, but this is one of the reasons that the board parser is strict. >>> It is immediately obvious what doesn't belong in the file. Also, in the >>> past users have tricked eeschema and pcbnew into a feature that didn't >>> technically exist by taking advantage of the loose file parsing. Then >>> when something changed internally and their clever hacks were broken, we >>> ended up with bug reports. I do not want to repeat this again. >>> >>>>> >>>>>> On 20 Mar 2018, at 10:19, Jeff Young <[email protected] >>>>>> <mailto:[email protected]> >>>>>> <mailto:[email protected]>> wrote: >>>>>> >>>>>> Hi Seth, >>>>>> >>>>>> The original version spit out log entries for skipped elements. This >>>>>> version follows the XML/browser convention of silently ignoring. >>>>>> Even though this isn’t an XML format, I think we need to recognise >>>>>> that we live in an XML world and XML processing conventions set >>>>>> expectations. >>> >>> After reading this, I am even more thankful that we didn't choose XML >>> for our file format. Gleefully ignoring file syntax that the parser >>> doesn't understand IMO is a bad idea. This flies in the face of the >>> basic premise that we control the format of our files not someone else. >>> As soon as you allow others to dictate your file format, you are in >>> trouble. >>> >>>>>> >>>>>> The patch strictly checks everything for round-tripping so that there >>>>>> is no data loss. The pad stuff is really a separate issue: it was >>>>>> meant to be loose only during development and then tightened up, but >>>>>> the tightening step was forgotten. Since we don’t store pad stuff we >>>>>> don’t understand, it has to be tightened. In short: if you can round >>>>>> trip stuff you don’t understand then do so; otherwise throw. >>> >>> The round tripping is fine and makes sense but it also adds to the code >>> complexity of the parser. The pad issue is different. I don't know >>> when that was slipped into the parser but it should not be there. If >>> there is an unexpected token, that should flag a file load error. >>> >>>>>> >>>>>> Certainly one use case is opening boards from future versions. If >>>>>> you edit them, then you’re at your own peril. This behaviour is >>>>>> common enough that I believe it is well understood (although we >>>>>> should obviously mention it in a version-check dialog). >>> >>> Perhaps, but I just see this ending badly. Maybe I'm being paranoid >>> here but it's so easy to imagine a scenario where someone did a lot of >>> editing in a newer version of kicad then makes the fatal mistake of >>> opening the board in an older version of kicad and wipes out a lot of >>> work. Technically not kicad's fault but I'm not so sure the user will >>> see it that way. >>> >>>>>> >>>>>> Another use case is 3rd-party tools (which might even be written as >>>>>> Python plugins) that want to carry their own stuff around in the >>>>>> board. These might even be processing/manufacturing instructions >>>>>> that don’t have any visual expression in Kicad anyway. >>> >>> This is one of the primary reasons that I do not like ignoring unknown >>> file formatting. It creates the potential for name space pollution that >>> could cause issues down the road. The eventual goal is to implement the >>> "property" token to define key/value pairs for third party applications >>> to add user specific information to any board object without polluting >>> the controlled part of the file format. The beauty of this is that we >>> do not have to coordinate with 3rd party developers to ensure we are not >>> clashing with anything the are working on. They are free to add >>> whatever properties they would like while we still maintain strict file >>> parsing. This was always part of the grand plan for the board file >>> format that kind of got lost in the noise and me becoming project leader. >>> >>> Cheers, >>> >>> Wayne >>> >>>>>> >>>>>> Cheers, >>>>>> Jeff. >>>>>> >>>>>> >>>>>>> On 19 Mar 2018, at 22:51, Seth Hillbrand >>>>>>> <[email protected] <mailto:[email protected]> >>>>>>> <mailto:[email protected]>> wrote: >>>>>>> >>>>>>> Hi Jeff- >>>>>>> >>>>>>> A few questions on how you are implementing this: >>>>>>> >>>>>>> 1) How does the user know what was skipped? I can imagine team >>>>>>> members with different versions getting into difficulty, especially >>>>>>> if the features being skipped change the board layout. >>>>>>> >>>>>>> 2) You are removing strict checking for most of the board but you >>>>>>> are adding strict checking for pad options. What's the difference? >>>>>>> >>>>>>> 3) If we keep these options around but don't allow editing/removing, >>>>>>> don't we run into a risk of generating a really wonk-y board that >>>>>>> only looks wonky in a newer version of KiCad but looks fine in an >>>>>>> older version? For example, imagine we implement rounded corners as >>>>>>> a parameter and then a user with an older version of KiCad edits and >>>>>>> saves the board. The rounded corner is only visible in KiCad 6 but >>>>>>> the user in KiCad 5 can edit the board to look right to her only to >>>>>>> have her colleague use KiCad 6 and see the track violating DRC. I >>>>>>> think that might be counter-intuitive for users but maybe there's a >>>>>>> way around it. >>>>>>> >>>>>>> In general, if I understand the use-case, this is to allow users to >>>>>>> open newer boards in older KiCad versions? Is there another use >>>>>>> case? >>>>>>> >>>>>>> I think I can see clear to this for options like the 3d-model offset >>>>>>> where it could be either inches or mm but there isn't a difference >>>>>>> in the actual layout. Allowing general unrecognized options would >>>>>>> seem to open up a worm can in terms of support as in "Please post >>>>>>> the KiCad version and the file version in order to figure out the >>>>>>> problem." >>>>>>> >>>>>>> -S >>>>>>> >>>>>>> >>>>>>> >>>>>>> 2018-03-18 9:46 GMT-07:00 Jeff Young <[email protected] >>>>>>> <mailto:[email protected]> >>>>>>> <mailto:[email protected]>>: >>>>>>> >>>>>>> OK, for your guys’ (re)viewing pleasure, a patch which >>>>>>> losslessly round-trips stuff it doesn’t understand. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On 16 Mar 2018, at 19:15, hauptmech <[email protected] >>>>>>>> <mailto:[email protected]> >>>>>>>> <mailto:[email protected]>> wrote: >>>>>>>> >>>>>>>> While i would still like to see this (my) shim go in, I agree >>>>>>>> with wayne. Until/unless a less brittle approach is used for >>>>>>>> the parser, it's better to signal a problem painfully and >>>>>>>> maintain the integrity of the file. >>>>>>>> >>>>>>>> I have to admit that when i first heard the s-expressions idea >>>>>>>> I assumed that the intention was to use a lisp like virtual >>>>>>>> machine for loading and maintaining design data. I've used vm's >>>>>>>> for data storage before (lua and python), and it's great. >>>>>>>> Parsing is free and you can jam in functions and other data >>>>>>>> when needed. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 17 Mar 2018 07:17, "Jeff Young" <[email protected] >>>>>>>> <mailto:[email protected]> >>>>>>>> <mailto:[email protected]>> wrote: >>>>>>>> >>>>>>>> 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 >>>>>>>> <[email protected] <mailto:[email protected]> >>>>>>>> <mailto:[email protected]>> 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 <[email protected] >>>>>>>>>>> <mailto:[email protected]> >>>>>>>> <mailto:[email protected]>> 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 >>>>>>>> <https://launchpad.net/~kicad-developers> >>>>>>>>>>> Post to : [email protected] >>>>>>>>>>> <mailto:[email protected]> >>>>>>>> <mailto:[email protected]> >>>>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers >>>>>>>> <https://launchpad.net/~kicad-developers> >>>>>>>>>>> More help : https://help.launchpad.net/ListHelp >>>>>>>> <https://help.launchpad.net/ListHelp> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> _______________________________________________ >>>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers >>>>>>>> <https://launchpad.net/~kicad-developers> >>>>>>>>>> Post to : [email protected] >>>>>>>>>> <mailto:[email protected]> >>>>>>>> <mailto:[email protected]> >>>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers >>>>>>>> <https://launchpad.net/~kicad-developers> >>>>>>>>>> More help : https://help.launchpad.net/ListHelp >>>>>>>> <https://help.launchpad.net/ListHelp> >>>>>>>>>> >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers >>>>>>>> <https://launchpad.net/~kicad-developers> >>>>>>>>> Post to : [email protected] >>>>>>>>> <mailto:[email protected]> >>>>>>>> <mailto:[email protected]> >>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers >>>>>>>> <https://launchpad.net/~kicad-developers> >>>>>>>>> More help : https://help.launchpad.net/ListHelp >>>>>>>> <https://help.launchpad.net/ListHelp> >>>>>>>> >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> Mailing list: https://launchpad.net/~kicad-developers >>>>>>>> <https://launchpad.net/~kicad-developers> >>>>>>>> Post to : [email protected] >>>>>>>> <mailto:[email protected]> >>>>>>>> <mailto:[email protected]> >>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers >>>>>>>> <https://launchpad.net/~kicad-developers> >>>>>>>> More help : https://help.launchpad.net/ListHelp >>>>>>>> <https://help.launchpad.net/ListHelp> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> Mailing list: https://launchpad.net/~kicad-developers >>>>>>> <https://launchpad.net/~kicad-developers> >>>>>>> Post to : [email protected] >>>>>>> <mailto:[email protected]> >>>>>>> <mailto:[email protected]> >>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers >>>>>>> <https://launchpad.net/~kicad-developers> >>>>>>> More help : https://help.launchpad.net/ListHelp >>>>>>> <https://help.launchpad.net/ListHelp> >>>>>>> >>>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> Mailing list: https://launchpad.net/~kicad-developers >>>>>> Post to : [email protected] >>>>>> <mailto:[email protected]> >>>>>> <mailto:[email protected]> >>>>>> Unsubscribe : https://launchpad.net/~kicad-developers >>>>>> More help : https://help.launchpad.net/ListHelp >>>>> >>>>> _______________________________________________ >>>>> Mailing list: https://launchpad.net/~kicad-developers >>>>> Post to : [email protected] >>>>> <mailto:[email protected]> >>>>> <mailto:[email protected]> >>>>> Unsubscribe : https://launchpad.net/~kicad-developers >>>>> More help : https://help.launchpad.net/ListHelp >>>> >>>> >>>> >>>> _______________________________________________ >>>> Mailing list: https://launchpad.net/~kicad-developers >>>> Post to : [email protected] >>>> <mailto:[email protected]> >>>> Unsubscribe : https://launchpad.net/~kicad-developers >>>> More help : https://help.launchpad.net/ListHelp >>>> >>> >>> _______________________________________________ >>> Mailing list: https://launchpad.net/~kicad-developers >>> Post to : [email protected] >>> <mailto:[email protected]> >>> Unsubscribe : https://launchpad.net/~kicad-developers >>> More help : https://help.launchpad.net/ListHelp >> >> _______________________________________________ >> Mailing list: https://launchpad.net/~kicad-developers >> Post to : [email protected] >> <mailto:[email protected]> >> Unsubscribe : https://launchpad.net/~kicad-developers >> More help : https://help.launchpad.net/ListHelp > _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
