-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Spike: Add a zipper to mutate Ast (WIP) #1148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| this.children.putAll(Assert.assertNotNull(children)); | ||
| } | ||
|
|
||
| public <T extends Node> List<T> getList(String key) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getChildrenWithName or just getChildren is a better namer I think. You know its a List from the return type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still stand by this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will change
|
|
||
| //TODO: Implement really in each Node | ||
| @Override | ||
| public ChildrenContainer getNamedChildren() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make it abstract and they will have to!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it was just temp
| @Override | ||
| public T withNewChildren(ChildrenContainer newChildren) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again make it abstract eventually to enforce this precondition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it was just temp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is actually in Node
| throw new IllegalStateException("children " + key + " is not a single value"); | ||
| } | ||
| return result.size() > 0 ? (T) result.get(0) : null; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getChildOrNull might ber a better name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, will change
| // // but at which index in the directives list | ||
| // } | ||
| // } | ||
| // } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this when you are done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
Implement Zipper support methods on all concrete Node classes
|
Work will continue here: #1335 |
No description provided.