[Pharo-project] PPElementParser & a small PetitTests contribution
While we're at it, I committed a tiny change to the assertions in PPAbstractParserTest, so that assertions return the parse result. I needed it for cases where I don't want to compare to an expected result, but still make further assertions than just "yup, it parses". For Coral I also wrote a simple PPDelegateParser subclass that applies its inner parser to the stream first element. It's like: PPPredicateObjectParser>>matching: elementParser message: aString ^ self on: [ :each | elementParser matches: each ] message: aString BUT: it will return the parse result of its inner parser, rather than the accepted element as-is. I use it to parse the argument array in a Coral script, some arguments are expected to be integers or other parseable. It allows me to predefine a bunch of utility parsers for simple argument types, that return a ready-to-use object. If you want to see the code, it's PPElementParser in the Coral package http://www.squeaksource.com/Coral.html but mostly it looks like this: parseOn: aStream | result | aStream atEnd ifTrue: [ ^ PPFailure message: 'element expected' at: aStream position ]. result := parser "end" parse: aStream uncheckedPeek. ^ result isPetitFailure ifFalse: [ aStream next. result ] ifTrue: [ PPFailure message: (message ifNil: ['error: ' , result printString , ' in element']) at: aStream position ] What do you think? I'm still not sure what the best way is. I hesitated between subclassing PPDelegateParser, PPLiteralParser⦠maybe a factory method in PPPredicateObjectParser would work as well: -- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
While we're at it, I committed a tiny change to the assertions in PPAbstractParserTest, so that assertions return the parse result. I needed it for cases where I don't want to compare to an expected result, but still make further assertions than just "yup, it parses".
Thank you, this is a great change.
For Coral I also wrote a simple PPDelegateParser subclass that applies its inner parser to the stream first element. It's like:
PPPredicateObjectParser>>matching: elementParser message: aString     ^ self         on: [ :each | elementParser matches: each ]         message: aString
BUT: it will return the parse result of its inner parser, rather than the accepted element as-is.
I use it to parse the argument array in a Coral script, some arguments are expected to be integers or other parseable. It allows me to predefine a bunch of utility parsers for simple argument types, that return a ready-to-use object.
If you want to see the code, it's PPElementParser in the Coral package http://www.squeaksource.com/Coral.html but mostly it looks like this:
parseOn: aStream     | result |     aStream atEnd ifTrue: [ ^ PPFailure message: 'element expected' at: aStream position ].     result := parser "end" parse: aStream uncheckedPeek.     ^ result isPetitFailure         ifFalse: [             aStream next.             result ]         ifTrue: [             PPFailure                 message: (message ifNil: ['error: ' , result printString , ' in element'])                 at: aStream position ]
What do you think? I'm still not sure what the best way is. I hesitated between subclassing PPDelegateParser, PPLiteralParser⦠maybe a factory method in PPPredicateObjectParser would work as well:
Do you have a ready made-image somewhere? I don't understand it from reading your mail only. Lukas -- Lukas Renggli www.lukas-renggli.ch
On 12 August 2011 07:21, Lukas Renggli <renggli@gmail.com> wrote:
Do you have a ready made-image somewhere? I don't understand it from reading your mail only.
Yup, get Pharo Coral from Jenkins: https://ci.lille.inria.fr/pharo/job/Pharo%20Coral/ -- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
I've committed PPElementParser + a couple factory methods on PPPredicateObjectParser On 12 August 2011 16:20, Damien Pollet <damien.pollet@gmail.com> wrote:
On 12 August 2011 07:21, Lukas Renggli <renggli@gmail.com> wrote:
Do you have a ready made-image somewhere? I don't understand it from reading your mail only.
Yup, get Pharo Coral from Jenkins: https://ci.lille.inria.fr/pharo/job/Pharo%20Coral/
-- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
-- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
Ohh ohhh, that was a bit too fast: I don't really see the point of PPElementParser? Why don't you just use aParser answer: anObject ? For the problems of your commit: - There is not a single test case. PetitParser had 99% test coverage before your commit. - All PPParser classes should have a class comment describing the class and its instance variables. - Please stick with the formatting conventions and don't leave commented code in the codebase. Cheers, Lukas On 13 August 2011 03:28, Damien Pollet <damien.pollet@gmail.com> wrote:
I've committed PPElementParser + a couple factory methods on PPPredicateObjectParser
On 12 August 2011 16:20, Damien Pollet <damien.pollet@gmail.com> wrote:
On 12 August 2011 07:21, Lukas Renggli <renggli@gmail.com> wrote:
Do you have a ready made-image somewhere? I don't understand it from reading your mail only.
Yup, get Pharo Coral from Jenkins: https://ci.lille.inria.fr/pharo/job/Pharo%20Coral/
-- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
-- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
-- Lukas Renggli www.lukas-renggli.ch
On 13 August 2011 09:05, Lukas Renggli <renggli@gmail.com> wrote:
I don't really see the point of PPElementParser? Why don't you just use
  aParser answer: anObject
something like this ? this temp feels strange⦠PPPredicateObjectParser class >> matching: elementParser message: aString | result | ^ (self on: [ :each | result := elementParser parse: each. result isPetitFailure not ] message: aString) answer: result -- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
On 13 August 2011 14:56, Damien Pollet <damien.pollet@gmail.com> wrote:
On 13 August 2011 09:05, Lukas Renggli <renggli@gmail.com> wrote:
I don't really see the point of PPElementParser? Why don't you just use
  aParser answer: anObject
something like this ? this temp feels strangeâ¦
PPPredicateObjectParser class >> matching: elementParser message: aString     | result |     ^ (self             on: [ :each |                 result := elementParser parse: each.                 result isPetitFailure not ]             message: aString)         answer: result
No. Wrapping a parser into a PPPredicateObjectParser really doesn't make sense. Can you give a real-world example that shows what you want to do? Do you just want to customize the error message if elementParser fails? Lukas -- Lukas Renggli www.lukas-renggli.ch
On 13 August 2011 15:06, Lukas Renggli <renggli@gmail.com> wrote:
Can you give a real-world example that shows what you want to do? Do you just want to customize the error message if elementParser fails?
I want to parse arrays of strings, while also parsing each element depending on where the array parse goes, e.g: #('command' '--option' '123') With a parser defined roughly like: (PPElementParser on: (#word asParser plus token end)) , (PPElementParser on: ('--' asParser , #word asParser plus token end ==> [:nodes | Option named: nodes second]) , (PPElementParser on: (#digit asParser plus token end ==> [:token | token value asNumber]) -- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
Ahh, I didn't understand that you try parse nested structures :-) I discussed with Doru at some point about that and I believe he intended to implement some support for it, but I don't know the current state. Maybe you want to check with him? Lukas On 13 August 2011 15:17, Damien Pollet <damien.pollet@gmail.com> wrote:
On 13 August 2011 15:06, Lukas Renggli <renggli@gmail.com> wrote:
Can you give a real-world example that shows what you want to do? Do you just want to customize the error message if elementParser fails?
I want to parse arrays of strings, while also parsing each element depending on where the array parse goes, e.g:
#('command' '--option' '123')
With a parser defined roughly like: (PPElementParser on: (#word asParser plus token end)) , Â Â (PPElementParser on: ('--' asParser , #word asParser plus token end ==> [:nodes | Option named: nodes second]) , Â Â (PPElementParser on: (#digit asParser plus token end ==> [:token | token value asNumber])
-- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
-- Lukas Renggli www.lukas-renggli.ch
On 13 August 2011 16:23, Lukas Renggli <renggli@gmail.com> wrote:
Ahh, I didn't understand that you try parse nested structures :-)
I discussed with Doru at some point about that and I believe he intended to implement some support for it, but I don't know the current state. Maybe you want to check with him?
I now realize that this should more or less work: #any asParser ==> [:node | elementParser parse: node] But in case of error it would return the inner petitfailure rather than a proper one about the position in the outer stream⦠-- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
On 13 August 2011 17:28, Damien Pollet <damien.pollet@gmail.com> wrote:
On 13 August 2011 16:23, Lukas Renggli <renggli@gmail.com> wrote:
Ahh, I didn't understand that you try parse nested structures :-)
I discussed with Doru at some point about that and I believe he intended to implement some support for it, but I don't know the current state. Maybe you want to check with him?
I now realize that this should more or less work:
#any asParser ==> [:node | elementParser parse: node]
But in case of error it would return the inner petitfailure rather than a proper one about the position in the outer streamâ¦
This is kind of ambiguous of how to report such nested errors. I wonder if you couldn't avoid all the troubles if you just used a flat string, instead of a preprocessed one? Lukas -- Lukas Renggli www.lukas-renggli.ch
On 13 August 2011 18:27, Lukas Renggli <renggli@gmail.com> wrote:
This is kind of ambiguous of how to report such nested errors. I wonder if you couldn't avoid all the troubles if you just used a flat string, instead of a preprocessed one?
ARGV is not a flat string, and I can't join it with spaces since I can't reproduce how the shell split it in the first place. -- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
On 13 August 2011 18:41, Damien Pollet <damien.pollet@gmail.com> wrote:
On 13 August 2011 18:27, Lukas Renggli <renggli@gmail.com> wrote:
This is kind of ambiguous of how to report such nested errors. I wonder if you couldn't avoid all the troubles if you just used a flat string, instead of a preprocessed one?
ARGV is not a flat string, and I can't join it with spaces since I can't reproduce how the shell split it in the first place.
What about a special sentinel like Character value: 0? Lukas -- Lukas Renggli www.lukas-renggli.ch
On 13 August 2011 09:05, Lukas Renggli <renggli@gmail.com> wrote:
Ohh ohhh, that was a bit too fast:
So what's the procedure in this situation ? Should I commit my patched PetitParser package to Coral's repo until it gets accepted? How will metacello deal with that? I have to update my config in the meantime? Should I package PPElementParser in one of Coral's packages and let you copy it later? But this is a recipe for getting load conflicts, isn't it? -- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
Should I commit my patched PetitParser package to Coral's repo until it gets accepted? How will metacello deal with that? I have to update my config in the meantime?
Did you patch PetitParser? AFAIK you didn't change anything, you just added a class and a few extension methods. Lukas -- Lukas Renggli www.lukas-renggli.ch
On 13 August 2011 18:29, Lukas Renggli <renggli@gmail.com> wrote:
Should I commit my patched PetitParser package to Coral's repo until it gets accepted? How will metacello deal with that? I have to update my config in the meantime?
Did you patch PetitParser? AFAIK you didn't change anything, you just added a class and a few extension methods.
I modified the package == patched it == new PetitParser-DamienPollet.XXX.mcz Question is, how/where do I commit so you can review/accept code into petitparser, but coral continues to load and work, and we don't get too many surprises when you decide to accept the new functionality. -- Damien Pollet type less, do more [ | ] http://people.untyped.org/damien.pollet
On 13 August 2011 18:43, Damien Pollet <damien.pollet@gmail.com> wrote:
On 13 August 2011 18:29, Lukas Renggli <renggli@gmail.com> wrote:
Should I commit my patched PetitParser package to Coral's repo until it gets accepted? How will metacello deal with that? I have to update my config in the meantime?
Did you patch PetitParser? AFAIK you didn't change anything, you just added a class and a few extension methods.
I modified the package == patched it == new PetitParser-DamienPollet.XXX.mcz
Question is, how/where do I commit so you can review/accept code into petitparser, but coral continues to load and work, and we don't get too many surprises when you decide to accept the new functionality.
Packaging separately seems to be the way to go. -- Lukas Renggli www.lukas-renggli.ch
participants (2)
-
Damien Pollet -
Lukas Renggli