stevedlawrence commented on code in PR #1367:
URL: https://github.com/apache/daffodil/pull/1367#discussion_r1868122527
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SchemaComponent.scala:
##########
@@ -330,19 +330,31 @@ final class Schema private (
}
/**
- * Given a name, retrieve the appropriate object.
+ * Given a qname, retrieve the appropriate object.
*
* This just scans each schema document in the schema, checking each one.
*/
- def getGlobalElementDecl(name: String) = {
+ def getGlobalElementDecl(qname: RefQName): Option[GlobalElementDecl] = {
Review Comment:
This function now is slightly differently than the other `getFoo` functions
below. That always gives me a pause when a bunch of things look like they
should be basically the same but have subtle differences. Why does this one
need to be different? Why is it okay for those to still lookup based on name,
but this one needs a qname?
##########
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala:
##########
@@ -695,12 +695,12 @@ abstract class TestCase(testCaseXML: NodeSeq, val parent:
DFDLTestSuite) {
}
def getRootNamespaceString() = {
- if (optExpectedOrInputInfoset.isDefined)
+ if (this.rootNSAttrib != "")
+ rootNSAttrib
Review Comment:
I wonder if rootNSAttrib wants to be an Option? So that `rootNS=""` is
`Some("")` explicitly means no-namespace, and if rootNS is missing then it's
None and we just search for any element with the name `root`, and errors if
there are ambiguities?
##########
daffodil-cli/src/test/scala/org/apache/daffodil/cli/cliTest/TestCLIParsing.scala:
##########
@@ -358,7 +358,7 @@ class TestCLIParsing {
runCLI(args"parse -s $schema -r unknown") { cli =>
cli.sendLine("12", inputDone = true)
- cli.expectErr("No root element found for unknown in any available
namespace")
+ cli.expectErr("No root element found for {}unknown")
Review Comment:
This is potentially confusing, and maybe not correct? `unknown` and
`{}unknown` are technically different things. The former means "some element
called unknown in any namespace". But the later means "some element called
unknown explicitly in no-namespace". The error message should probably not
include the curly braces here.
##########
daffodil-test/src/test/resources/org/apache/daffodil/section06/namespaces/namespaces.tdml:
##########
@@ -910,7 +910,7 @@
<tdml:document><![CDATA[333]]></tdml:document>
<tdml:errors>
<tdml:error>Schema Definition Error</tdml:error>
- <tdml:error>More than one definition for name: vagueEle</tdml:error>
+ <tdml:error>More than one definition for name: {}vagueElem</tdml:error>
Review Comment:
Similar question here, this feel incorrect since `{}` explicity means
no-namespace, but no-namespace was not suggested as the namespace to use for
this test. It's even possible that `vagueElement` doesn't even exist with
no-namespace, it chould have two different namespaces, in which case {} is
confusing since it implies no-namespace.
Maybe related, I noticed that `RootSpec.toString` always outputs curly
braces. Maybe that's where the curly braces are coming from? Maybe it should
only output curly braces if ns is defined? That would better match our
"extended syntax".
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SchemaDocument.scala:
##########
@@ -261,9 +262,16 @@ final class SchemaDocument private (xmlSDoc:
XMLSchemaDocument)
lazy val defineVariables = annotationObjs.collect { case dv:
DFDLDefineVariable => dv }
/**
- * by name getters for the global things that can be referenced.
+ * by name/qname getters for the global things that can be referenced.
*/
- def getGlobalElementDecl(name: String) = globalElementDecls.find { _.name ==
name }
+ def getGlobalElementDecl(qname: RefQName): Option[GlobalElementDecl] =
Review Comment:
Same question as before, why is this RefQName when the other similar
functions are just a String?
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SchemaSet.scala:
##########
@@ -356,7 +356,15 @@ final class SchemaSet private (
ge
}
case RootSpec(None, rootElementName) => {
- findRootElement(rootElementName)
+ val possibleRoots: Seq[GlobalElementDecl] = schemaSet.schemas.flatMap {
+ _.schemaDocuments.flatMap(_.searchGlobalElementDecl(rootElementName))
+ }
+ if (possibleRoots.length == 1) {
+ possibleRoots.head
+ } else {
+ val qn = RefQName(None, rootElementName, NoNamespace)
Review Comment:
> So we assume not specifying a namespace URI may mean the intention
I'm not sure that's a safe assumption. It's possible the user just didn't
realize there were two elements with the same name and they actually did want
the namespaced one. Feels like it would be safer to not assume and instead
error saying there's an ambiguity and just require that the user be explicit
about which namespace they want. It believe it's always possible to set
no-namespace if that is needed for example one can use `{}foo` in the CLI. Is
there a way to do it in the TDML Runner? Maybe `rootNS=""` is explicitly
no-namespace?
--
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]
