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

BaseStreamWriter.writeSpace(String) should not close open element #95

Open
kwin opened this issue Nov 30, 2019 · 5 comments
Open

BaseStreamWriter.writeSpace(String) should not close open element #95

kwin opened this issue Nov 30, 2019 · 5 comments
Labels
good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project

Comments

@kwin
Copy link
Contributor

kwin commented Nov 30, 2019

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 (

) . 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 ...

@cowtowncoder
Copy link
Member

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.

@kwin
Copy link
Contributor Author

kwin commented Dec 1, 2019

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 (

public final static String P_OUTPUT_UNDERLYING_WRITER = "com.ctc.wstx.outputUnderlyingWriter";
).

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.

@cowtowncoder
Copy link
Member

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 cowtowncoder added the good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project label Dec 2, 2019
@kwin
Copy link
Contributor Author

kwin commented Dec 9, 2019

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.

@cowtowncoder
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project
Projects
None yet
Development

No branches or pull requests

2 participants