Call reset on reusable SAXParser instance
#742
Conversation
| def parser: SAXParser = parserInstance.get | ||
| def parser: SAXParser = { | ||
| val p = parserInstance.get | ||
| p.reset() |
There was a problem hiding this comment.
Auto-reset on get leads to surprising changes in behavior, e.g. non-persistence of the settings on XMLLoader.reader, as witnessed by the changes to the test above.
Better place for auto-reset seems to be FactoryAdapter.loadDocument(), but it will then apply to all readers, not just the default one...
Better still would be - in my opinion - not to reuse parsers, readers and factory adapters and not to use thread-locals.
There was a problem hiding this comment.
I am not going to campaign for reverting it, but I am not a fan ;)
| override def warning(e: SAXParseException): Unit = gotAnError = true | ||
| override def error(e: SAXParseException): Unit = gotAnError = true | ||
| override def fatalError(e: SAXParseException): Unit = gotAnError = true | ||
| }) | ||
| try { | ||
| XML.loadString("<a>") | ||
| XML.adapter.loadDocument(Source.fromString("<a>"), reader) |
There was a problem hiding this comment.
I'd love to make XMLLoader.adapter, NoBindingFactoryAdapter and FactoryAdapter private at some point (and remove some of them altogether), and add XMLLoader.loadDocument(inputSource: InputSource, reader: XMLReader)...
It's really more of a guess... In a non-public codebase we have an Thanks for taking a look. |
Discussion: #176 (comment)