tuxji commented on code in PR #911:
URL: https://github.com/apache/daffodil/pull/911#discussion_r1068422556
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/ConvertTextStandardNumberParser.scala:
##########
@@ -34,7 +32,8 @@ import org.apache.daffodil.processors.Success
import org.apache.daffodil.processors.TermRuntimeData
import org.apache.daffodil.processors.TextNumberFormatEv
-import java.lang.{Long => JLong, Float => JFloat, Double => JDouble, Number =>
JNumber}
+import java.lang.{Float => JFloat, Number => JNumber, Long => JLong, Double =>
JDouble}
Review Comment:
Should these be on separate lines?
##########
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesTextNumber.scala:
##########
@@ -186,41 +305,57 @@ trait ConvertTextNumberMixin {
* but cannot be used as an operational pattern given that
* potentially lots of things have been removed from it.
*/
- final protected lazy val patternStripped = {
+ final protected lazy val patternNoQuoted = {
Review Comment:
Did you mean `patternNoQuotes`, which would tell the programmer more clearly
that all quotes will be removed, not just one pair of quotes?
##########
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesTextNumber.scala:
##########
@@ -80,14 +99,72 @@ object TextNumberPatternUtils {
// don't forget the ^ and $ (start of data, end of data) because we want
// the match to consume all the characters, starting at the beginning.
val vPattern=
-
s"""^${posPrefix}${posBeforeV}V${posAfterV}${posSuffix}(?:;${negPrefix}${negBeforeV}V${negAfterV}${negSuffix})?$$"""
+ s"""^${posPrefix}${posBeforeV}V${posAfterV}${posSuffix}""" ++
+ s"""(?:;${negPrefix}${negBeforeV}(?:V${negAfterV}${negSuffix})?)?$$"""
Review Comment:
I also noticed that this long-line change uses ++ (the sequence append
method) to concatenate the two strings together. Usually we use + (the string
concat method) to concatenate two strings in Scala, same as in Java. Using ++
works, but it looks surprising to programmers like me and the Scala compiler
may generate less optimal code.
##########
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesTextNumber.scala:
##########
@@ -275,17 +410,67 @@ case class ConvertTextStandardNumberPrim(e: ElementBase)
extends Terminal(e, true)
with ConvertTextNumberMixin {
+
final override protected lazy val textDecimalVirtualPointFromPattern: Int = {
- val r = TextNumberPatternUtils.vregexStandard
- r.findFirstMatchIn(patternStripped) match {
- case Some(r(_, _, afterV, _, _, _, _, _)) => afterV.length
- case None =>
- e.SDE(
- s"""The dfdl:textNumberPattern '%s' contains 'V' (virtual decimal
point).
- | Other than the sign indicators, it can contain only
- | '#', then digits 0-9 then 'V' then digits 0-9.
- | The positive part of the dfdl:textNumberPattern is
mandatory.""".stripMargin('|'),
+ if (hasV) {
+ val r = TextNumberPatternUtils.vRegexStandard
+ r.findFirstMatchIn(patternNoQuoted) match {
+ case Some(r(_, _, afterV, _, _, _, _, _)) => afterV.length
+ case None =>
+ e.SDE(
+ s"""The dfdl:textNumberPattern '%s' contains 'V' (virtual decimal
point).
+ | Other than the sign indicators, it can contain only
+ | '#', then digits 0-9 then 'V' then digits 0-9.
+ | The positive part of the dfdl:textNumberPattern is
mandatory.""".stripMargin('|'),
+ pattern)
+ }
+ } else if (hasP) {
+ val rr = TextNumberPatternUtils.pOnRightRegexStandard
+ val rl = TextNumberPatternUtils.pOnLeftRegexStandard
+ val rightMatch = rr.findFirstMatchIn(patternNoQuoted)
+ val leftMatch = rl.findFirstMatchIn(patternNoQuoted)
+ (leftMatch, rightMatch) match {
+ case (None, None) => e.SDE(
+ """The dfdl:textNumberPattern '%s' contains 'P' (virtual decimal point
positioners).
+ |However, it did not match the allowed syntax which allows the
sign indicator
+ |plus digits on only one side of the P symbols.""".stripMargin,
pattern)
Review Comment:
|'s don't line up with """ (could also start the first line with | and align
the rest of the |'s to the first |).
--
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]
