Fix indented writer misaligning End tags after Comment + Text

Changelog.md

@@ -61,6 +61,10 @@ The MSRV has been raised to 1.86.
 
 ### Bug Fixes
 
+- [#276]: Fixed `Writer::new_with_indent` producing misaligned closing tags
+  when an element contained a non-text child (e.g. a comment) followed by a
+  text node. The `</tag>` was appended directly after the text instead of
+  starting on a new line at the correct indentation level.
 - [#977]: `NamespaceResolver::push` (and hence every `NsReader` `Start`/`Empty`
   event) now returns the new `NamespaceError::TooDeeplyNested` when a document
   nests elements deeper than `u16::MAX`, instead of overflowing the internal
@@ -85,6 +89,7 @@ The MSRV has been raised to 1.86.
   `decode_and_unescape_value_with()`. Use `normalized_value()` and
   `normalized_value_with()` instead.
 
+[#276]: https://github.com/tafia/quick-xml/issues/276
 [#963]: https://github.com/tafia/quick-xml/pull/963
 [#977]: https://github.com/tafia/quick-xml/issues/977
 [#980]: https://github.com/tafia/quick-xml/issues/980

src/writer.rs

@@ -260,8 +260,10 @@ impl<W: Write> Writer<W> {
     /// Writes the given event to the underlying writer.
     pub fn write_event<'a, E: Into<Event<'a>>>(&mut self, event: E) -> io::Result<()> {
         let mut next_should_line_break = true;
+        let mut should_mark_non_text = true;
         let result = match event.into() {
             Event::Start(e) => {
+                should_mark_non_text = false;
                 let result = self.write_wrapped("<", &e, ">");
                 if let Some(i) = self.indent.as_mut() {
                     i.grow();
@@ -271,6 +273,9 @@ impl<W: Write> Writer<W> {
             Event::End(e) => {
                 if let Some(i) = self.indent.as_mut() {
                     i.shrink();
+                    if i.last_non_text_level > i.current_indent_len {
+                        i.should_line_break = true;
+                    }
                 }
                 self.write_wrapped("</", &e, ">")
             }
@@ -285,11 +290,13 @@ impl<W: Write> Writer<W> {
             ),
             Event::Text(e) => {
                 next_should_line_break = false;
+                should_mark_non_text = false;
                 self.write(e.as_bytes())
             }
             Event::Comment(e) => self.write_wrapped("<!--", &e, "-->"),
             Event::CData(e) => {
                 next_should_line_break = false;
+                should_mark_non_text = false;
                 self.write(b"<![CDATA[")?;
                 self.write(e.as_bytes())?;
                 self.write(b"]]>")
@@ -301,6 +308,9 @@ impl<W: Write> Writer<W> {
             Event::Eof => Ok(()),
         };
         if let Some(i) = self.indent.as_mut() {
+            if should_mark_non_text {
+                i.mark_non_text_content();
+            }
             i.should_line_break = next_should_line_break;
         }
         result
@@ -679,6 +689,10 @@ pub(crate) struct Indentation {
     indents: String,
     /// The current amount of indentation
     current_indent_len: usize,
+    /// Indent level at which the most recent non-text event was written.
+    /// Used by End to force a line break when a non-text child (Comment,
+    /// Empty, child Start, etc.) preceded a Text event in the same element.
+    last_non_text_level: usize,
 }
 
 impl Indentation {
@@ -690,6 +704,7 @@ impl Indentation {
             indent_size,
             indents: std::iter::repeat(indent_char).take(128).collect(),
             current_indent_len: 0,
+            last_non_text_level: 0,
         }
     }
 
@@ -721,4 +736,8 @@ impl Indentation {
             self.indents.push(self.indent_char);
         }
     }
+
+    fn mark_non_text_content(&mut self) {
+        self.last_non_text_level = self.current_indent_len;
+    }
 }

src/writer/async_tokio.rs

@@ -11,8 +11,10 @@ impl<W: AsyncWrite + Unpin> Writer<W> {
     /// Writes the given event to the underlying writer. Async version of [`Writer::write_event`].
     pub async fn write_event_async<'a, E: Into<Event<'a>>>(&mut self, event: E) -> Result<()> {
         let mut next_should_line_break = true;
+        let mut should_mark_non_text = true;
         let result = match event.into() {
             Event::Start(e) => {
+                should_mark_non_text = false;
                 let result = self.write_wrapped_async("<", &e, ">").await;
                 if let Some(i) = self.indent.as_mut() {
                     i.grow();
@@ -22,17 +24,22 @@ impl<W: AsyncWrite + Unpin> Writer<W> {
             Event::End(e) => {
                 if let Some(i) = self.indent.as_mut() {
                     i.shrink();
+                    if i.last_non_text_level > i.current_indent_len {
+                        i.should_line_break = true;
+                    }
                 }
                 self.write_wrapped_async("</", &e, ">").await
             }
             Event::Empty(e) => self.write_wrapped_async("<", &e, "/>").await,
             Event::Text(e) => {
                 next_should_line_break = false;
+                should_mark_non_text = false;
                 self.write_async(e.as_bytes()).await
             }
             Event::Comment(e) => self.write_wrapped_async("<!--", &e, "-->").await,
             Event::CData(e) => {
                 next_should_line_break = false;
+                should_mark_non_text = false;
                 self.write_async(b"<![CDATA[").await?;
                 self.write_async(e.as_bytes()).await?;
                 self.write_async(b"]]>").await
@@ -44,6 +51,9 @@ impl<W: AsyncWrite + Unpin> Writer<W> {
             Event::Eof => Ok(()),
         };
         if let Some(i) = self.indent.as_mut() {
+            if should_mark_non_text {
+                i.mark_non_text_content();
+            }
             i.should_line_break = next_should_line_break;
         }
         result

tests/writer-indentation.rs

@@ -1,4 +1,4 @@
-use quick_xml::events::{BytesStart, BytesText, Event};
+use quick_xml::events::{BytesEnd, BytesStart, BytesText, Event};
 use quick_xml::writer::Writer;
 
 use pretty_assertions::assert_eq;
@@ -550,3 +550,30 @@ mod in_attributes_multi {
         );
     }
 }
+
+// Regression test for https://github.com/tafia/quick-xml/issues/276
+// Elements containing both a comment and a text node should not produce
+// misaligned closing tags.
+#[test]
+fn issue_276_comment_then_text_indent() {
+    let mut buffer = Vec::new();
+    let mut writer = Writer::new_with_indent(&mut buffer, b' ', 4);
+
+    writer
+        .write_event(Event::Start(BytesStart::new("tag2")))
+        .unwrap();
+    writer
+        .write_event(Event::Comment(BytesText::new("Test comment")))
+        .unwrap();
+    writer
+        .write_event(Event::Text(BytesText::new("Test")))
+        .unwrap();
+    writer
+        .write_event(Event::End(BytesEnd::new("tag2")))
+        .unwrap();
+
+    let output = std::str::from_utf8(&buffer).unwrap();
+    // The closing </tag2> must start on a new line at the same indentation
+    // level as <tag2>, not immediately after the text node.
+    assert_eq!(output, "<tag2>\n    <!--Test comment-->Test\n</tag2>");
+}