Factory::build should return Result<XPath> instead of Result<Option<XPath>>

[leoschwarz]

Since this would be a breaking change I guess it could also be left as is at this point (it's just a minor suggestion). But maybe it would be more ergonomic for API users if they only had to check the result once? I'm aware that in the parser module the Result<Option<...>> serves its purpose but to the outside world it would be probably more convenient if there was just a Result containing the useful value directly and if there really is a need for differentiation between errors to provide an enum.

I'm not completely sure what None would indicate – an empty string? – but maybe it would be possible to just convert it into an instance of the current ParserError inside Factory::build?

[leoschwarz]

Ok(None) is the result of parsing an empty string as I realized later.

I can't find any information, whether an empty XPath query is valid in the first place, but I think that it might be sensible to remove the Factory struct (unless there are plans for extension and having additional methods in there in the future) and replace it with a module level function.

[shepmaster]

whether an empty XPath query is valid in the first place

You know, it's probably not...

$ echo '<a/>' | xmllint --xpath '' -
XPath error : Invalid expression
xmlXPathEval: evaluation failed
XPath evaluation failure

I'll probably leave the Factory struct as it can reuse some internal data structures, but having the module function makes sense.