kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

Looks interesting.  Coding policy issues aside, I have a few questions
and comments:

I'm assuming this doesn't change the fp-lib-table file formatting.

Why not make the PARSER object just work directly on an istream object
or some abstraction that wraps input stream objects (not always
necessarily based on std::istream)?  It really doesn't matter where the
input stream comes from.  This just makes the parser more flexible.  You
are only supporting string and file input streams at the moment.  There
are a few wxInputStream objects
(http://docs.wxwidgets.org/3.0/classwx_input_stream.html) that could be
useful.  I don't want to limit the input stream objects to just the few
that the standard c++ library provides and I would rather not write our
own if we don't have to.

There needs to be a SEXPR_TYPE_BOOL for boolean types.  AFIAR, the
current parser supports yes, no, 0, 1, true, and false as boolean types.
 I'm not sure there are any other types.

Why are you not throwing an exception when there is a invalid key when
parsing the fp-lib-table file?  Would there be any reason to accept an
invalid file?

Have you done any speed comparisons against the current parser design?
I could live with a slight increase in parsing time although I would
rather not.

On 9/9/2016 11:21 AM, Mark Roszko wrote:
> If you want a peek at something I've played with:
> 
> https://github.com/marekr/kicad-sexpr/commit/3bf73b984eaeef3d8aede351592a2cc07677cb10
> 
> 
> I avoid boost like the plague ^^
> 
> On Fri, Sep 9, 2016 at 10:55 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>> I understand completely.  I've written enough of our sexpr code to know
>> how it works and where it's strengths and weaknesses lie.  I just want
>> to make sure broken file formats are flagged as such by any parsers that
>> we write.  If our output formatters are creating these files, that is an
>> all together different issue that needs to be addressed.
>>
>> At some point, if I ever get the time, I would like to review this.  It
>> might be useful use a property tree like parser/formatter such as how
>> xml is handled or some other similar construct rather than the way we
>> are doing it now.  I took a look at the boost::property_tree class it
>> looks like a possible candidate.  I was hoping to do something like this
>> for the new schematic file format.  It's primarily a issue of finding
>> the free time to learn something new at the moment.
>>
>> On 09/09/2016 10:38 AM, Mark Roszko wrote:
>>> ....the existing parser is "by the seat of our pants" style parsing
>>> where there is no centralized parsing logic and instead
>>> manual definition of expected tokens for every single parsable atom
>>> and well, this has led to places where expectRightParenthesis() isn't
>>> called and the like. But that won't break anything because it
>>> continues walking down the token list after the case for an atom is
>>> executed.
>>>
>>> Meh, I probably suck at explaining ^
>>>
>>> Looks like I forgot to submit a patch for these files as I submitted
>>> one for another template file before that I think JP commited. Because
>>> my strict SEXPR parser barked :3
>>>
>>> On Fri, Sep 9, 2016 at 10:19 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>>> Hmm.  I took another look at this patch and I'm concerned as to why the
>>>> page layout template parser did not choke on this.  That should be fixed
>>>> as well.
>>>>
>>>> On 9/8/2016 2:56 PM, Werner Almesberger wrote:
>>>>> The default and logo page layout templates are missing some opening
>>>>> parentheses. Eeschema's parser accepts them anyway, but it tripped
>>>>> my s-expr parser.
>>>>>
>>>>> The gost templates and the built-in default in
>>>>> common/page_layout/page_layout_default_description.cpp
>>>>> are both correct.
>>>>>
>>>>> - Werner
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> 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
>>>
>>>
>>>
> 
> 
>