tuxji commented on code in PR #799:
URL: https://github.com/apache/daffodil/pull/799#discussion_r886085077
##########
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala:
##########
@@ -2364,6 +2361,35 @@ sealed abstract class DocumentPart(part: Node, parent:
Document) {
}
}.trim.toUpperCase()
+ /*
+ * Description: Compare ByteDocumentPart/BitsDocumentPart string to a string
of valid characters.
+ * If the string runs into non-valid characters, skip over them.
+ * If the string runs into "//", any character following it will
be skipped (including valid ones).
+ * Characters on a new line will be considered again, unless
they are found after a "//"
+ * Parameters: String containing the acceptable characters (0,1 for bits)
(0-9,A-F,a-f for bytes)
Review Comment:
Yes, I also prefer functional implementations where you pass in everything,
they return everything to you as a result, and they don't modify any mutable
state (unless you pass it in as an argument too).
##########
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala:
##########
@@ -2364,6 +2361,35 @@ sealed abstract class DocumentPart(part: Node, parent:
Document) {
}
}.trim.toUpperCase()
+ /*
+ * Description: Compare ByteDocumentPart/BitsDocumentPart string to a string
of valid characters.
+ * If the string runs into non-valid characters, skip over them.
+ * If the string runs into "//", any character following it will
be skipped (including valid ones).
+ * Characters on a new line will be considered again, unless
they are found after a "//"
+ * Parameters: String containing the acceptable characters (0,1 for bits)
(0-9,A-F,a-f for bytes)
+ * Return: ListBuffer[Char] of the accepted characters from the
ByteDocumentPart/BitsDocumentPart element
+ */
+ def commentCompatibleFormat(validCharacters: String) : ListBuffer[Char] = {
+ val tempListBuffer = new ListBuffer[Char]()
+ var noCommentBool = true
+
+ for (i <- 0 to partRawContent.length()-1){
Review Comment:
Yes, I agree with performing more strict validation of the document part
contents (any invalid character except whitespace must be in a comment or must
throw an exception).
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/UnitTestTDMLRunner.scala:
##########
@@ -345,6 +345,46 @@ class UnitTestTDMLRunner {
assertEquals(expected, actual)
}
+ @Test def testCommentBit(): Unit = {
+ val xml = <document bitOrder="LSBFirst">
+ <documentPart type="bits">00000010 //this is a
label111</documentPart>
Review Comment:
[DAFFODIL-2369](https://issues.apache.org/jira/browse/DAFFODIL-2369) is
worth reading to answer these questions. // to end of line is only one of two
C/C++ comment markers DAFFODIL-2369 suggests - I strongly suggest also
implementing the /* */ convention in order to avoid weird syntax highlighting
in Scala code like this file. DAFFODIL-2369 also says breaking up the data
into separate documentPart elements in order to use standard XML comments is
not effective enough due to the clutter involved.
--
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]
