adds ParentAwareVisitor for implementation that needs to know all parent AST nodes (Visitee) #39
Conversation
parent_visitor.go
Outdated
|
|
||
| func (v *ParentAwareVisitor) pop() { | ||
| if len(v.Parents) == 0 { | ||
| log.Fatal("no parent to pop") |
There was a problem hiding this comment.
This isn't very lib-like - most people don't use the log package, and this is an uncatchable os.Exit call.
This single line would prevent me from using all this code.
There was a problem hiding this comment.
you are right. actually this was debug code added to get a better hint at what went wrong when building it. I think it can be removed altogether.
| // ParentAwareVisitor keeps track of all the parents of each AST node | ||
| // while visiting all the elements and options. | ||
| type ParentAwareVisitor struct { | ||
| Parents []Visitee |
There was a problem hiding this comment.
It might be nice to have LastParent Visitee which is nil if there is no parent, otherwise has the last element of the Parents list, checking if LastParent != nil is nicer than len(Parents) > 0 and then getting the last parent with Parents[len(Parents)-1]
There was a problem hiding this comment.
Ah, actually disregard potentially, if you have #38 it's fine.
There was a problem hiding this comment.
thinking about dropping #38 in favour of this.
parent_visitor.go
Outdated
| return &ParentAwareVisitor{delegate: delegate} | ||
| } | ||
|
|
||
| func (v *ParentAwareVisitor) push(parent Visitee) { |
There was a problem hiding this comment.
nit: i would put these at the bottom
moved formatter and all commands to proto-contrib package
Support required groups
…or more newlines" This reverts commit 1ed9ecb.
…field.Elements
Add c-style comment testing
|
see #68 instead |
No description provided.