stevedlawrence commented on a change in pull request #130: Initial commit
URL: https://github.com/apache/incubator-daffodil/pull/130#discussion_r229766128
##########
File path:
daffodil-lib/src/main/scala/org/apache/daffodil/cookers/EntityReplacer.scala
##########
@@ -642,6 +642,9 @@ sealed abstract class ListOfStringLiteralBase(
protected def cook(raw: String, context: ThrowsSDE, forUnparse: Boolean):
List[String] = {
val rawList = raw.split("\\s+").toList
+ if (((rawList(0) == "") || (rawList.last == "")) && (rawList.length != 1))
+ context.SDE("The string \"\" is not allowed at the start or end of the
list if it is not the only item in the list")
+
Review comment:
I tested the split functionality:
```scala
scala> " foo ".split("\\s+")
res0: Array[String] = Array("", foo)
```
So this only detects leading whitespace, but not trailing whitespace. Maybe
instead just check ``raw.head.isWhitespace`` and ``raw.last.isWhitespace`` and
throw an SDE if either is true. Should definitely add a test for trailing
whitespace.
Also, from a users perspective, I'm not sure if the error message is clear.
For example, if the user had ``dfdl:terminator=" FOO"``, they would get an
error about an empty string starting/ending not being allowed, but the property
is not an empty string. So it might not be clear how a user should fix this. It
might be more helpful to just mention that the property cannot start or end
with whitespace. And maybe even mention that "%SP;" might be what the user
intended? The StringLiteralBase has an error message for whitespace, maybe
something similar to that for consistency?
----------------------------------------------------------------
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
