woodstox icon indicating copy to clipboard operation
woodstox copied to clipboard

BaseStreamWriter.writeSpace(String) should not close open element

Open kwin opened this issue 4 years ago • 5 comments

Sometimes it is useful to also indent attributes within an element. For that org.codehaus.stag2.writeSpace(String) should be used. Unfortunately that will always close any open start tags so it cannot be used to indent attributes as internally it calls writeText (https://github.com/FasterXML/woodstox/blob/fee31c2efae3858635a2af212dcad09b250a84be/src/main/java/com/ctc/wstx/sw/BaseStreamWriter.java#L1175) . As the javadoc is clearly stating this should be used for indentation (http://fasterxml.github.io/stax2-api/javadoc/3.1.4/org/codehaus/stax2/XMLStreamWriter2.html#writeSpace(java.lang.String)) it is unexpected that this behaves like writeRaw(String) as this prevents it being used for indenting attributes. Any subsequent calls of writeAttribute(...) lead to exception

javax.xml.stream.XMLStreamException: Trying to write an attribute when there is no open start element.
	at com.ctc.wstx.sw.BaseStreamWriter.throwOutputError(BaseStreamWriter.java:1589)
	at com.ctc.wstx.sw.SimpleNsStreamWriter.writeAttribute(SimpleNsStreamWriter.java:73)
	at ...

kwin avatar Nov 30 '19 20:11 kwin

Hmmh. Indentation mentioned was meant for for element indentation, and not for changing possible white-space between attributes. And writeSpace() specifically would only produce cdata sections, which inter-attribute white space would not be. So this would be usage different from what was intended originally. Use of writeRaw() however should be supported for such case.

But if you or anyone is interested in extending things to work for this usage as well, I think that would make sense.

cowtowncoder avatar Dec 01 '19 06:12 cowtowncoder

Use of writeRaw() however should be supported for such case.

Unfortunately not. If I call the following methods, I get the exception from above

writeStartTag(...)
writeRaw(...)
writeAttribute(...)

This is due to the fact that writeRaw(...) closes the start tag implicitly and therefore afterwards does no longer allow attributes to be added. The only workaround right now is to use the underlying writer directly to write the attribute indent (https://github.com/FasterXML/woodstox/blob/fee31c2efae3858635a2af212dcad09b250a84be/src/main/java/com/ctc/wstx/api/WstxOutputProperties.java#L184).

That feels wrong and IMHO the javadoc should state this limitation more explicitly in writeSpace(...) or in the best case this limitation should be lifted.

kwin avatar Dec 01 '19 09:12 kwin

We are always open for patches: writeRaw() I think should not modify the state -- and should be usable for this usage -- for sure. Whether writeSpace() should work that way or not I don't have strong opinion, but would accept a patch to change too. Similarly for javadoc changes.

I don't have time to work on this myself in near-term but I think this could be something easy for others to pick up so will label as such.

cowtowncoder avatar Dec 02 '19 19:12 cowtowncoder

writeRaw() I think should not modify the state

This is tricky from a backwards-compatibility point of view as the API javadoc explicity says

It will not update state of the writer (except by possibly flushing output of previous writes, like finishing a start element)

(http://fasterxml.github.io/stax2-api/javadoc/3.1.4/org/codehaus/stax2/XMLStreamWriter2.html#writeRaw(java.lang.String)) Maybe adding an explicit flag whether start tags should be finished could be added as overloaded writeRaw() method.

kwin avatar Dec 09 '19 08:12 kwin

Good point; it is bit of ambiguous case since without flushing like that possibly existing use cases could break.

Not sure how to proceed here, however, since as a practical matter I do not think I want to start making changes to Stax2 API just for this use case.

A likely workaround here might just be to instead use underlying Writer but first flushing XMLStreamWriter.

cowtowncoder avatar Dec 10 '19 00:12 cowtowncoder