jadams-tresys commented on a change in pull request #169: Fix incorrect line
numbers and ensure XML errors include line numbers
URL: https://github.com/apache/incubator-daffodil/pull/169#discussion_r249933776
##########
File path:
daffodil-lib/src/main/scala/org/apache/daffodil/xml/DaffodilConstructingLoader.scala
##########
@@ -86,46 +86,60 @@ class DaffodilConstructingLoader(uri: URI, errorHandler:
org.xml.sax.ErrorHandle
source
}, true) {
- override def reportSyntaxError(pos: Int, msg: String) {
+ // This one line is a bit of a hack to get consistent line numbers. The
+ // scala-xml libary reads XML from a scala.io.Source which maintains private
+ // line/col information about where in the Source we are reading from (i.e.
+ // scala.io.Source.pos). The problem is that when CDATA or a processing
+ // instruction is encountered, the library switches to a custom
+ // "WithLookAhead" scala.io.Source that buffers the original Source. This
+ // lookahead Source allows it to peek ahead a few characters, which is used
+ // to find the end of CDATA and processing instructions. The problem is that
+ // when it switches to this new Source, we lose position information since
+ // that information is private to each Source. This causes line information
+ // to reset to zero when the first CDATA or processing instruction is found.
+ // And there is no good way to copy position information from one source to
+ // another. So, what we can do is call this lookahead() function before any
+ // XML is parsed. This causes the ConstructingLoader to immediately switch to
+ // the buffering source. There may be some slight overhead for buffering, but
+ // at last our line numbers are correct.
+ lookahead()
+
+
+ private def makeSAXParseException(pos: Int, msg: String) = {
val line = Position.line(pos)
val col = Position.column(pos)
val exc = new org.xml.sax.SAXParseException(msg, null, uri.toString, line,
col)
+ exc
+ }
+
+ override def reportSyntaxError(pos: Int, msg: String) {
+ val exc = makeSAXParseException(pos, msg)
errorHandler.error(exc)
}
- /**
- * We probably aren't supposed to override mkAttributes(), however,
- * mkAttributes is a really really easy place to add our file/line/col
- * attributes and namespace, since this is the function in the
- * ConstructingParser that creates that stuff. However, the super mkAttribute
- * function, and functions that lead up to it being called, change the pos
- * member variable. So by the time our mkAttributes is called, the pos is
- * off, usually by a line or two, which is confusing. So override xTag, which
- * is called early enough before the position is changed, to capture the
- * position, and then use that position when mkAttributes is called.
+ /*
+ * Callback method invoked by MarkupParser after parsing an element, between
+ * the elemStart and elemEnd callbacks. This adds daffodil file/line/column
+ * information as attributes to the existing input attrs, modifying the scope
+ * if necessary, then creates an element using the super def elem function.
*
- * Note that this is potentially fragile and could break if scala-xml ever
- * makes significant changes. However, these functions, and the classes
- * containing these functions have not been modified for many years, so I
- * suspect they are fairly stable and it's safe to assume things won't change
- * significantly.
+ * @param pos the position in the source file
+ * @param pre the prefix
+ * @param local the local name
+ * @param attrs the attributes (metadata)
+ * @param scaope the namespace binding scope
Review comment:
Spelling error on scope
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
