search

RazrFalcon / roxmltree

Represent an XML document as a read-only tree.
Apache License 2.0
434 stars 37 forks source link

Replace &'a str by &'a Cow<'input, str> where this is the backing data type. #92

Closed adamreichold closed 1 year ago

adamreichold commented 1 year ago

I started converting return types from &'a str to &'a Cow<'input, str> and there is indeed some loss ergonomics as calling code must call .map(AsRef::as_ref) to e.g. compare against Option<&str>.

We could work around this by providing *_cow methods returning Option<&'a Cow<'input, str>> and implement the existing methods as .map(AsRef::as_ref) on top of those so that no implementations need to be duplicated. But that would imply a significant increase in API surface for purely technical reasons.

Another issue that should however be solvable by adding another internal type is that ExpandedName does double duty a) holding references to expanded names from within the document to returned from Node::tag_name which would need to store &'a Cow<'input, str> and b) holding values passed into the API for matching purposes like Node::has_tag_name which need to store &'c str with an unrelated lifetime 'c.

Closes #88

RazrFalcon commented 1 year ago

I understand the performance benefits, but the new API is ugly as hell. I haven't looked into it myself, so I'm not sure if there a better ways, but I would rather leave everything as is.