jadams-tresys commented on PR #812: URL: https://github.com/apache/daffodil/pull/812#issuecomment-1185693570
I've made some cleanup changes for removing SAX from the CLI tool, but I didn't address the following from Mike B: Ex: there should be no code in DataProcessor associated with particular SAX handlers, and no base classes or traits one must derive from in order to use our SAX API. I think the requirements were not clear, and SAX is for some reason fairly confusing. But there are things we need only for unit testing that are in the primary libraries. Ex: Class DaffodilParseXMLReader and Class DaffodilUnparseContentHandler are defined as if they are part of our API, when they should just be under src/test. DaffodilParseXMLReader and DaffodilUnparseContentHandler to me seem like they might be necessary as I believe they are responsible for translating the various SAX events into something that Daffodil understands. I feel like these, or something similar, are necessary in order for someone to use Daffodil with SAX. I feel like it shouldn't be required of someone using the API to have to define something for Daffodil to be able understand SAX events. I do think the API could be smoothed out in this regard, where the DaffodilParseXMLReader and DaffodilUnparseContentHandler aren't really exposed by the API. For example, when parsing we are currently happy to accept a users ContentHandler, but then the user needs to call DFDL.DataProcessor.newXMLReaderInstance to get the current processors XMLReader, and then set that XMLReader's ContentHandler to the one they are trying to pass into Daffodil. To me this feels pretty clunky and could be hidden, but I haven't looked at this long enough to come up with a cleaner solution. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
