On 2011-08-11, at 15:26, Tudor Girba wrote:
> Hi Camillo,
>
> A first-class omit would be better especially given that you could package it
> as an extension.
>
> In any case, the current code should be reverted. While you play with another
> implementation, could you move the current versions from the PetitParser
> repository?
sure sure remove it :)
> Cheers,
> Doru
>
>
> On 11 Aug 2011, at 15:14, Camillo Bruni wrote:
>
>> Thanks for the vast answers and pointing out critical parts of the
>> implementation.
>>
>> On 2011-08-11, at 08:14, Tudor Girba wrote:
>>> Hi,
>>> Camillo, thanks for taking an interest and for letting us know that you
>>> committed.
>>>
>>> Lukas, thanks for the answer. I would also like to add one comment: omit is
>>> maybe interesting for small and quick parsers in which you have the grammar
>>> and the parser together. However, for anything significant, if you want to
>>> split the grammar and the various parsers, it all of a sudden becomes less
>>> useful because you do not want to restrict the grammar.
>>
>> Yeah and that's exactly the reason why I think this is useful. Since we want
>> to use PetitParser also for small "dirty" projects where portability is a
>> secondary problem. And I still disagree that it becomes less useful, it is
>> just a way of filtering your output from parser in a very simple way ;).
>> Furthermore having the omit doesn't mean that you will have to use it. t
>> mean there is also the #flatten message which goes somewhat into the same
>> direction IMO, since it is there for modifying directly in the parser the
>> data. It could be omitted as well and put into the grammar.
>>
>>> I also took a quick look at the implementation. There are simply too many
>>> ifs that add complexity to an already non-trivial code.
>>
>> Indeed the implementation is not very nice, and more of a preliminary hack
>> to see if it works, I will definitely do a second pass with lukas'
>> suggestion for a first-class object to denote the omit property.
>>
>>> All in all, I think we should retire this add-on.
>>>
>>> Cheers,
>>> Doru
>>>
>>> On 11 Aug 2011, at 07:31, Lukas Renggli wrote:
>>>> Hi Camillo,
>>>>> I quickly hacked (with a lot of tests) support for omit in PetitParser.
>>>>> This should simplify some parsing efforts as an additional filtering step
>>>>> could be avoided.
>>>>>
>>>>> parser := ($" asParser omit, $" asParser negate star flatten, $" asParser
>>>>> omit)
>>>>> parser parse: '"asdfsd"' "yields directly #('asdfsd') "
>>>>>
>>>>> vs
>>>>>
>>>>> parser := ($" asParser, $" asParser negate star flatten, $" asParser)
>>>>> parser parse: '"asdfsd"'
>>>>>
>>>>> which yields #($" 'asdfsd' $")
>>>>
>>>> I don't really see use of #omit. The element is still in a collection
>>>> and you need to extract it from there. What is the benefit other than
>>>> it is now at index 1 instead of 2?
>>>>
>>>> Also the implementation has some problems:
>>>>
>>>> 1. #omit makes it really hard to compose grammars, because parsers
>>>> suddenly get very asymmetric and start to affect each others behavior
>>>> depending on how you compose them.
>>
>> it shouldn't be about the behavior of the grammes, I might have not done
>> everything correctly in my implementation, but the way the parsing rules are
>> evaluation is not changed, its should be only affecting the output of the
>> parsers (in this sense a special PPActionParser).
>>
>>>> 2. For the same reason grammar transformation becomes impossible, one
>>>> of the design goals of PetitParser.
>>
>> sure, but having the feature doesn't mean you have to use it. And in my case
>> it would simply some of the parsers I use.
>> Actually in the end the Omit is nothing but a special PPActionParser right?
>> (Just a curious question, how do you handle grammar transformation in this
>> case?)
>>
>>>> 3. #omit makes a typical grammar (the smalltalk one) roughly 15%
>>>> slower without even using the new feature.
>>
>> yep thats an implementation issue, as you mentioned addressing this with a
>> first-class object won't affect existing grammars at all.
>>
>>>> 4. There are various methods (#-, #match..., #copy..., ...) that need
>>>> to be fixed when you add state to a parser.
>>>>
>>>> 5. There is no need for eager (#initialize) as well as lazy
>>>> initialization (kills the possibility for a quick invocation).
>>
>> right, thats indeed an ugly property of my hack (=> first class objects will
>> do)
>>
>>>> 6. Please do not remove PPParser class>>new and do not change how
>>>> initialization works, otherwise you break GemStone, VisualWorks, and
>>>> GNU Smalltalk. The Seaside wiki has some nice documentation of how to
>>>> correctly initialize objects (basically following the pattern of
>>>> Objective-C).
>>
>> ok, that was a bit hasty
>>
>>>> I think, problems 1, 3 (!), 4, 5 and to some extent also 2 could be
>>>> solved if the implementation was a bit more object-oriented. That is,
>>>> if you added a PPOmitParser that would dispatch from #parseOn: to some
>>>> other parsing method #parseOmittedOn: you wouldn't need all those
>>>> #ifTrue:ifFalse:.
>>>>
>>>>> I pushed my solution to the petitparser repos. I didn't add full support
>>>>> for flatten.
>>>>
>>>> Did you see #map: and #permutation:? Both allow you do the same in a
>>>> less invasive way:
>>
>> Of course in my example this is straight forward, but in a bit more complex
>> parser with a couple of options or intermediate tokens you don't want it
>> would be nicer to directly specify it on the parser nodes. And in such a
>> sense only affect how flatten works...
>>
>>>> parser := ($" asParser , $" asParser negate star flatten, $" asParser).
>>>> parser map: [ :begin :inner :end | inner ]
>>>>
>>>> or
>>>>
>>>> parser := ($" asParser , $" asParser negate star flatten, $" asParser).
>>>> parser permutation: #(2)
>>>>
>>>> Additionally, did you see the experimental binding/capturing
>>>> functionality in PetitBeta? This kind of provides something much more
>>>> general and (I believe) much more powerful without the above
>>>> drawbacks. Bind-Capture allows you to bind certain parse results
>>>> anywhere down the tree to names and directly access them later on in a
>>>> capture block. It also deals with unpacking and reshuffling
>>>> collections automatically, check the tests for that.
>>>>
>>>> Your example would look like:
>>>>
>>>> parser := ($" asParser , ($" asParser negate star flatten bind:
>>>> #inner) , $" asParser).
>>>> parser capture: [ :inner | inner ]
>>
>>
>> Ah cool, this is indeed a bit nicer, didn't have at the new branch yet… will
>> do ;)
>>
>>
>
> --
> www.tudorgirba.com
>
> "Some battles are better lost than fought."
>
>
>
>