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

Need support for XPath callbacks #39

Closed
jbowtie opened this issue Sep 22, 2013 · 2 comments
Closed

Need support for XPath callbacks #39

jbowtie opened this issue Sep 22, 2013 · 2 comments

Comments

@jbowtie
Copy link
Contributor

jbowtie commented Sep 22, 2013

I'm currently working on this in a branch, but it's an invasive design change so I'm creating an issue to solicit feedback before creating a pull request. It will result in a new xml.Node call and new xpath interface.

If variable names or non-built in functions are included in an XPath, the evaluator invokes callback functions in order to resolve them. Currently the context object we create leaves them null and exposes no way to set them.

I had to go away and do some research on callbacks, so here's what I think needs to happen (and is mostly implemented in my not-yet-published branch):

  • Create xpath.VariableScope interface. The ResolveVariable and ResolveFunction functions defined by this interface do the actual resolution.
  • Export functions that match the callback definitions. The void* pointer can be unpacked to a VariableScope variable and the appropriate resolve functions invoked.
  • Create corresponding C functions in the xpath.go header that take an XPathContext and void pointer and set the appropriate bits in the XPathContext.
  • Add XPath.SetVariableResolution(v VariableScope) which calls the C function, passing unsafe.Pointer(&v) as the data.
  • Finally, define Node.SearchWithVariables. This takes an extra VariableScope parameter and calls XPath.SetVariableResolution.
  • Create a simple struct that wraps a map and implements VariableScope. This is needed for testing.

Just typing this out has clarified a couple of details I was unsure about, so it's helped in that regard. Please provide any feedback on the design; I'm sure I'll get plenty of feedback on the actual code once I've created a pull request.

@mdayaram
Copy link
Contributor

Thanks for the support! These are definitely really cool much needed features! Unfortunately we're pretty swamped for the next couple of weeks and these changes are not trivial, so I won't really be able to provide much feedback until our release cycle is over (next two weeks). Please hold on tight until then!

@jbowtie
Copy link
Contributor Author

jbowtie commented Oct 15, 2013

Closing as this is now implemented by pull request #44

@jbowtie jbowtie closed this as completed Oct 15, 2013
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

No branches or pull requests

2 participants