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 a structure to facilitate .as_str()/.as_bytes() #4

Closed
matt2xu opened this issue Feb 26, 2016 · 8 comments
Closed

Add a structure to facilitate .as_str()/.as_bytes() #4

matt2xu opened this issue Feb 26, 2016 · 8 comments

Comments

@matt2xu
Copy link

matt2xu commented Feb 26, 2016

At the moment, Element has two methods as_bytes() and as_str() to allow access to raw bytes or conversion to UTF-8 strings. Also, Element has no way to access the all contents (from start to end) as either raw or str, only as String (which is incidentally why the writer cannot use write_wrapped_str for start elements). Attributes, on the other hand, iterates over raw keys with converted strings. Maybe we could introduce a "Text" structure or something similar, that:

  • automatically converts to [u8](this is what the Deref trait allows if I understand correctly)
  • as a as_str() method to do the explicit conversion
  • then Attributes would iterate over (Text, Text), and Element could have two functions name() that returns a Text over the bytes from start to name_end, and content() that returns a Text over the bytes from start to end.

Not sure if Text is a good name, but you get the idea :-)

@tafia
Copy link
Owner

tafia commented Feb 26, 2016

LGTM!

@matt2xu
Copy link
Author

matt2xu commented Feb 26, 2016

I tried that, but it turned out to be too verbose and complicated. I went a different route: use only [u8] for name() and content() methods, and use an AsStr trait that implements as_str() for [u8]. Not sure if the trait is worth to have or not. Let me know what you think: matt2xu@44a245b

@tafia
Copy link
Owner

tafia commented Feb 27, 2016

I don't know how useful is the content method, I can only see the scenario where we want to copy the entire element, perhaps we can have Element deref to &[u8] so we can copy_from_slice directly on &Element. But I'm not sure it won't interfere with some other methods ...

@matt2xu
Copy link
Author

matt2xu commented Feb 27, 2016

The content method is useful to get the whole element, including attributes, for instance in the writer (there is no longer a different method for writing start elements) and for the Debug impl. Plus it seems more natural to get the content of a Text or Comment Event rather than their name, it makes the intent of the code clearer in my opinion.

@tafia
Copy link
Owner

tafia commented Feb 28, 2016

I think content is ok.
Can you do a PR ?

@matt2xu
Copy link
Author

matt2xu commented Feb 28, 2016

Sure!

@tafia
Copy link
Owner

tafia commented Feb 29, 2016

closed by #6

@tafia tafia closed this as completed Feb 29, 2016
@tafia
Copy link
Owner

tafia commented Feb 29, 2016

For people having compilation issue, #6 breaks existing code. To fix you'll need to:

  • replace as_bytes with name
  • replace as_str with content().as_str() or name().as_str() depending on your use case
  • for attributes, you'll need to call as_str on values (which used to be a &str before Improve API for creating new elements #3)

dralley pushed a commit to dralley/quick-xml that referenced this issue May 20, 2022
Fix attribute parsing and provide information about attribute shapes
dralley pushed a commit to dralley/quick-xml that referenced this issue May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants