Basic implementation of the ElementWriter concept #278

dralley · 2021-03-20T05:02:02Z

@tafia Here's a basic implementation of the idea. See the tests I added for examples of how using it would look.

I can add documentation and improved tests once the general idea is solidified. There's probably things to be improved :)

src/writer.rs

dralley · 2021-03-21T14:46:48Z

I was also thinking about a design that would allow arbitrary depth, but I wasn't sure if it could be done without allocations.

tafia

Thanks!
I would suggest returning the &mut Writer<W> in write_text/write_empty for people to write more things in the same expression.

tafia · 2021-03-22T01:31:53Z

src/writer.rs

+
+pub struct NodeWriter<'a, W: Write> {
+    writer: &'a mut Writer<W>,
+    tag_name: &'a [u8],


You don't need to keep a separate field for tag_name as you can get it from the BytesStart::name.

Sorry didn't read your comment on lifetimes.

Yup. It's possible there's a way around it that I don't know about though.

Not only was there a way, it was actually documented perfectly.

quick-xml/src/events/mod.rs

Lines 96 to 115 in 4c85384

/// Converts the event into a borrowed event. Most useful when paired with [`to_end`].

///

/// # Example

///

/// ```

/// # use quick_xml::{Error, Writer};

/// use quick_xml::events::{BytesStart, Event};

///

/// struct SomeStruct<'a> {

/// attrs: BytesStart<'a>,

/// // ...

/// }

/// # impl<'a> SomeStruct<'a> {

/// # fn example(&self) -> Result<(), Error> {

/// # let mut writer = Writer::new(Vec::new());

///

/// writer.write_event(Event::Start(self.attrs.to_borrowed()))?;

/// // ...

/// writer.write_event(Event::End(self.attrs.to_end()))?;

/// # Ok(())

src/writer.rs

dralley · 2021-03-22T02:13:27Z

src/writer.rs

+            .create_node(b"paired")
+            .with_attribute(("attr1", "value1"))
+            .with_attribute(("attr2", "value2"))
+            .write_text(BytesText::from_plain_str("text"))


Should a write_cdata be added?

Yes ... ideally all types of events

All events, or just the of types which would make sense to be nested inside a single pair of tags?

In my mind, this is just an optional high level API with a focus on being easy to use, and difficult to misuse. If you wanted to reproduce "weird" or invalid XML perfectly you would probably want to use the existing API instead, which provides total control.

I think of all the event types the only ones that make sense inside a pair of tags are Text, CData, and maybe PI? Comment, Start, End, Decl, and EOF would all be invalid.

decl probably doesn't make any sense indeed

dralley · 2021-03-22T03:21:42Z

I would suggest returning the &mut Writer in write_text/write_empty for people to write more things in the same expression.

Could you give a code example of what this would look like? ("writing more things in the same expression")

tafia · 2021-03-22T04:16:31Z

Could you give a code example of what this would look like? ("writing more things in the same expression")

 writer
            .create_node(b"paired")
            .with_attribute(("attr1", "value1"))
            .with_attribute(("attr2", "value2"))
            .write_text(BytesText::from_plain_str("text"))?
            .create_node(b"paired")  // another node
            .with_attribute(("attr1", "value1"))
            .with_attribute(("attr2", "value2"))
            .write_text(BytesText::from_plain_str("text"))?
            .write_event(...)? // directly write an event

It is not really better. On the other hand we're not losing anything from returning a &mut self.

dralley · 2021-03-22T20:20:57Z

src/writer.rs

+    }
+
+    #[test]
+    fn element_writer_scope() {


I wanted to try this experiment, but I'm not strongly attached to it. What do you think? It seems useful for really simple cases of nesting, but less so more complex ones.

Multiple levels of nesting would get messy.

dralley · 2021-03-24T03:08:19Z

@tafia This is ready for re-review, although I should add more tests before it actually merges. Just let me know if there's anything objectionable about the API.

Here's what a larger piece of code looks like when rewritten to use this strategy: dralley/rpmrepo_metadata@0498173

tafia · 2021-03-25T02:54:12Z

Looks good to me, let me know when you want to merge.
Thanks!

dralley · 2021-03-25T04:52:27Z

src/writer.rs

+    }
+
+    /// Write some text inside the current element.
+    pub fn write_text_content(self, text: BytesText) -> Result<&'a mut Writer<W>> {


Does a pair of tags with no text always get represented as [Start] [Text ""] [End]?

What do you mean? If possible I think we could optimize it and write an empty event but this is really not very important.

I just wanted to be sure that what I'm doing is correct - if it was ever necessary to write two separate start and end events with no text event (even an empty one) in the middle, this wouldn't be convenient. But if there needs to always be one (unless writing an empty tag, in which case you have better options than this function), then this is fine.

dralley · 2021-03-25T04:54:07Z

@tafia Apart from the question, and your approval of the docstrings I just added, I think this is good to go.

dralley · 2021-03-25T04:55:41Z

src/writer.rs

+    }
+
+    /// Create a new scope for writing XML inside the current element.
+    pub fn write_nested_content<F>(mut self, closure: F) -> Result<&'a mut Writer<W>>


Would inner or scoped or any other word make more sense here?

inner seems better. Not a fan of scoped I think.

src/events/mod.rs

dralley · 2021-03-26T02:55:28Z

src/events/mod.rs

@@ -315,10 +315,13 @@ impl<'a> BytesStart<'a> {
    }

    /// Try to get an attribute
-    pub fn try_get_attribute(&'a self, attr_name: &[u8]) -> Result<Option<Attribute<'a>>> {
+    pub fn try_get_attribute<N: AsRef<[u8]> + Sized>(


Attribute names don't need to be escaped right? This would let users pass &str, &[u8], or Vec<u8>.

right, names don't need escaping.

I noticed one difference between a library I'm writing and an existing one that uses libxml2 - the one based on libxml2 seems to strip tab characters \t out of attribute values. I can't find anything in the spec that mentions that nor can I find it in libxml2 though - do you know whether that might be required?

Add an API that simplifies writing basic tag pairs and small amounts of content nesting in a more declarative fashion.

tafia · 2021-03-30T03:55:58Z

Great! Thanks a lot for your time.

dralley force-pushed the simple-nested-write branch 2 times, most recently from 018b26c to a205039 Compare March 20, 2021 05:07

dralley mentioned this pull request Mar 20, 2021

Edition2018 #274

Merged

dralley commented Mar 20, 2021

View reviewed changes

src/writer.rs Outdated Show resolved Hide resolved

dralley force-pushed the simple-nested-write branch from a205039 to f11d580 Compare March 20, 2021 18:40

dralley commented Mar 20, 2021

View reviewed changes

src/writer.rs Show resolved Hide resolved

tafia requested changes Mar 22, 2021

View reviewed changes

dralley commented Mar 22, 2021

View reviewed changes

src/writer.rs Show resolved Hide resolved

dralley commented Mar 22, 2021

View reviewed changes

src/writer.rs Show resolved Hide resolved

dralley commented Mar 22, 2021

View reviewed changes

dralley force-pushed the simple-nested-write branch from f11d580 to 96057a0 Compare March 22, 2021 19:23

dralley changed the title ~~Basic implementation of the NodeWriter concept~~ Basic implementation of the ElementWriter concept Mar 22, 2021

dralley force-pushed the simple-nested-write branch 2 times, most recently from 1614465 to d8a4797 Compare March 22, 2021 20:02

dralley commented Mar 22, 2021

View reviewed changes

dralley force-pushed the simple-nested-write branch 11 times, most recently from 79e1ac1 to 4c11c60 Compare March 24, 2021 03:02

dralley mentioned this pull request Mar 24, 2021

RPM metadata library drahnr/cantaloupe#1

Open

tafia approved these changes Mar 25, 2021

View reviewed changes

dralley force-pushed the simple-nested-write branch 4 times, most recently from cdd8ba7 to 404e87d Compare March 25, 2021 04:49

dralley commented Mar 25, 2021

View reviewed changes

dralley commented Mar 26, 2021

View reviewed changes

src/events/mod.rs Outdated Show resolved Hide resolved

dralley force-pushed the simple-nested-write branch from 2dd8dae to e0602af Compare March 26, 2021 02:20

dralley commented Mar 26, 2021

View reviewed changes

dralley force-pushed the simple-nested-write branch from e0602af to b24cb9a Compare March 27, 2021 14:05

dralley added 2 commits March 27, 2021 10:24

Add a high level API for writing XML

ed42367

Add an API that simplifies writing basic tag pairs and small amounts of content nesting in a more declarative fashion.

Let try_get_attribute() use either &str or &[u8]

1e02176

dralley force-pushed the simple-nested-write branch from b24cb9a to 1e02176 Compare March 27, 2021 14:24

dralley mentioned this pull request Mar 30, 2021

Easy element construction with body #265

Open

tafia merged commit d00f9ca into tafia:edition2018 Mar 30, 2021

dralley deleted the simple-nested-write branch March 30, 2021 03:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic implementation of the ElementWriter concept #278

Basic implementation of the ElementWriter concept #278

dralley commented Mar 20, 2021 •

edited

Loading

dralley commented Mar 21, 2021

tafia left a comment

tafia Mar 22, 2021

tafia Mar 22, 2021

dralley Mar 22, 2021

dralley Mar 24, 2021

dralley Mar 22, 2021

tafia Mar 22, 2021

dralley Mar 22, 2021 •

edited

Loading

tafia Mar 23, 2021

dralley commented Mar 22, 2021 •

edited

Loading

tafia commented Mar 22, 2021

dralley Mar 22, 2021 •

edited

Loading

dralley commented Mar 24, 2021 •

edited

Loading

tafia commented Mar 25, 2021

dralley Mar 25, 2021

tafia Mar 27, 2021

dralley Mar 27, 2021 •

edited

Loading

dralley commented Mar 25, 2021 •

edited

Loading

dralley Mar 25, 2021

tafia Mar 27, 2021

dralley Mar 27, 2021

dralley Mar 26, 2021

tafia Mar 27, 2021

dralley Feb 22, 2022

tafia commented Mar 30, 2021

	/// Converts the event into a borrowed event. Most useful when paired with [`to_end`].
	///
	/// # Example
	///
	/// ```
	/// # use quick_xml::{Error, Writer};
	/// use quick_xml::events::{BytesStart, Event};
	///
	/// struct SomeStruct<'a> {
	/// attrs: BytesStart<'a>,
	/// // ...
	/// }
	/// # impl<'a> SomeStruct<'a> {
	/// # fn example(&self) -> Result<(), Error> {
	/// # let mut writer = Writer::new(Vec::new());
	///
	/// writer.write_event(Event::Start(self.attrs.to_borrowed()))?;
	/// // ...
	/// writer.write_event(Event::End(self.attrs.to_end()))?;
	/// # Ok(())

Basic implementation of the ElementWriter concept #278

Basic implementation of the ElementWriter concept #278

Conversation

dralley commented Mar 20, 2021 • edited Loading

dralley commented Mar 21, 2021

tafia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dralley Mar 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dralley commented Mar 22, 2021 • edited Loading

tafia commented Mar 22, 2021

dralley Mar 22, 2021 • edited Loading

Choose a reason for hiding this comment

dralley commented Mar 24, 2021 • edited Loading

tafia commented Mar 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dralley Mar 27, 2021 • edited Loading

Choose a reason for hiding this comment

dralley commented Mar 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tafia commented Mar 30, 2021

dralley commented Mar 20, 2021 •

edited

Loading

dralley Mar 22, 2021 •

edited

Loading

dralley commented Mar 22, 2021 •

edited

Loading

dralley Mar 22, 2021 •

edited

Loading

dralley commented Mar 24, 2021 •

edited

Loading

dralley Mar 27, 2021 •

edited

Loading

dralley commented Mar 25, 2021 •

edited

Loading