kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #01815
Re: Patch for specctra session import bug
-
To:
kicad-devel@xxxxxxxxxxxxxxx
-
From:
"Brian Sidebotham" <brian.sidebotham@...>
-
Date:
Wed, 17 Sep 2008 16:36:38 +0100
-
In-reply-to:
<48D11703.9020403@...>
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