moovweb/gokogiri

Need support for XPath callbacks

jbowtie opened this issue · 2 comments

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.

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!

Closing as this is now implemented by pull request #44