Skip to content
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

Add esquery.traverse #45

Closed
wants to merge 1 commit into from
Closed

Add esquery.traverse #45

wants to merge 1 commit into from

Conversation

kumavis
Copy link
Contributor

@kumavis kumavis commented Apr 24, 2015

Playing with esquery I found I wanted richer information than a list of matches (e.g. parents, etc)

I broke out the traversing and matching so you get a callback for each found node. I suppose this is a little more dangerous as you could accidently change the ast in such a way selector that would have been is not matched.

Let me know what you think, happy to make adjustments.

That said it looks like I'm passing the wrong parent here:
visitor(ancestry[k], parent, ancestry.slice());

@michaelficarra
Copy link
Member

I like it. This obviously needs tests, though. Why are you cloning the ancestry list every time?

@kumavis
Copy link
Contributor Author

kumavis commented Apr 25, 2015

I am cloning the ancestry list so the consumer doesn't modify it and break stuff. Perhaps we can leave that up to them. Since you are already compiling the selector yourself to use this, maybe it can be assumed that things are fragile.

@michaelficarra
Copy link
Member

Yeah, if the consumer wants to break things, let them. We shouldn't take the huge perf hit of cloning the ancestry list just to protect them from themselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants