← Back to team overview

kicad-developers team mailing list archive

Re: Patch for specctra session import bug

 

2008/9/17 Dick Hollenbeck <dick@...>:
> Brian Sidebotham wrote:
>> Hi,
>>
>> Attached is a small patch that obviously fixes my specific issues with
>> having a number as a module name (0805). I tested that this then
>> successfully lets pcbnew import a specctra session that has numerical
>> module names.
>>
>> This may or may not be acceptable, but it fixed my problem :)
>>
>> Best Regards,
>>
>> Brian Sidebotham.
>>
>
> Brian,
>
> Since this function is used in so many places, and its purpose in the
> first place was to enforce grammar, I think it is too much risk to make
> this change within this function. Can you find the place to add the
> tok == T_NUMBER test outside this function to solve your problem,
> something like this
>
>
> if( isSymbol(tok) || tok == T_NUMBER )
>
>
> That way we do not break the filtering effect of the parser. The parser
> is a very general specctra parser and has applications outside of Kicad
> as written, and I'd rather not break it for this specific use case.
> isSymbol() is used everywhere so a behavioral change to it would need
> more study than I have time for at the moment.
>
> I did check the specctra grammar and use of a number for a component_id
> does seem to be allowed by the grammar, so your desire to fix the
> problem is acceptable, I'd just hope we can fix only that problem and
> not break others.
>
> It should be pretty easy to find the handler on where to move this test
> to, because of the doCOMPONENT() type naming used in the recursive
> descent design.
>
>
> Then I would submit such a patch.

Hi Dick,

Thanks for your pointers. At first I had assumed that isSymbol was
only used for components, but it is indeed used in all sorts of other
places. I have attached a second patch which moves this test to
doCOMPONENT.

I again tested this with my board and the session is correctly
imported with this patch.

Thanks for checking the Specctra grammar as I do not have that to hand.

Best Regards,

Brian Sidebotham.
 ------=_Part_18575_4451478.1221665798253 Content-Type: application/octet-stream; name=specctra.patch
Content-Transfer-Encoding: base64
X-Attachment-Id: f_fl8483ff0
Content-Disposition: attachment; filename=specctra.patch

[Attachment content not displayed.] ------=_Part_18575_4451478.1221665798253-- 




Follow ups

References