Pup 6841 document parser api #8
Conversation
hlindberg
force-pushed
the
PUP-6841_document-parser-api
branch
2 times, most recently
from
d845e05 to
e27a286
Compare
| class MyChecker | ||
| attr_reader :acceptor | ||
| def initialize(diagnostics_producer) | ||
| @@bad_word_visitor ||= Puppet::Pops::Visitor.new(nil, "badword", 0, 0) |
There was a problem hiding this comment.
The concept of the visitor/polymorphic dispatcher must be explained. To go from this line what actually happens (calls to badword_QualifiedName etc.) is not self evident.
There was a problem hiding this comment.
Added some additional text, but mainly referred to the blog post I wrote 2014.
| check_bad_word(model) | ||
|
|
||
| # Then check all of its content | ||
| model.eAllContents.each {|m| check_bad_word(m) } |
There was a problem hiding this comment.
This exposes Rgen. It might be of value to refrain from doing that in examples if we want to get rid of Rgen later on.
There was a problem hiding this comment.
Don't think we have an alternative to eAllContents that is available from 3.8 future parser and forward?
And I think we should say that only eAllContent is API to reduce the amount of exposure to RGen API that we may not want to keep in an optimized implementation,
| result = result.model | ||
|
|
||
| # validate using the default validator and get hold of the acceptor containing the result | ||
| acceptor = parser.validate(result) |
There was a problem hiding this comment.
Perhaps make the validate method capable of validating the result from parse_string rather than the model that it contains. That way, the result becomes opaque (i.e. no need to disclose the somewhat unintuitive implementation detail result = result.model ).
There was a problem hiding this comment.
Sure, but the fact that it returns a Factory for further manipulation remains and a user of the API needs to know as they are bound to stumble over that otherwise,
(Potentially a lot of stuff (tests) to rewrite I think if we do not return a Factory, so don't think that is a good option either even if it is simple to add a Factory wrapper if one is needed).
| # | ||
| puts "Standard validation errors found: #{acceptor.error_count}" | ||
| puts "Standard validation warnings found: #{acceptor.warning_count}" | ||
|
|
There was a problem hiding this comment.
I would start with this example since it shows how to do a complete parse/validate using what's "in the box". Once that's covered, it's easier to understand that the objective with the MyValidation module is to go beyond that. It's also easier to understand how things fit together.
There was a problem hiding this comment.
Ok, will do a better "set up" before presenting all of the example code.
There was a problem hiding this comment.
I did the setup in the document rather than in the runnable code
Added link to blog post about polymorphic dispatch.
This puts the example use before the example implementation to make it easier to understand the purpose of the implementation.
|
|
||
| The result of parsing is an instance of `Puppet::Pops::Model::Factory`, on which there is one public method named `model` that returns the Abstract Syntax Tree (AST) that represents the parsed source. The root of this tree is always an instance of | ||
| `Puppet::Pops::Model::Program`. This class, along with the other close to 90 classes that make up all kinds of expressions and statements in the puppet language are found in the file `lib/puppet/pops/model/model_meta.rb` | ||
|
|
There was a problem hiding this comment.
The above changes in Puppet 5.0.0 so that there is no need to unwrap the returned factory using the call to model. We added a backwards compatible method which still allows calling model on the return value, that then just returns self.
|
This PR needs an update for Puppet 5.0.0 since some of the methods (like |
|
Thanks for writing this up @hlindberg! The detailed example is a huge help. I'd love to hear how this could be adapted for Puppet 5 when you have a chance. |
PR contains a draft of the documentation (to end up somewhere else) and an example that makes use of the API.
PR made to make it easier to review and comment.