[GitHub] [daffodil] mbeckerle commented on a diff in pull request #900: Implement textNumberPattern 'V' virtual decimal point.

[GitBox <gi...@apache.org>]

mbeckerle commented on code in PR #900:
URL: https://github.com/apache/daffodil/pull/900#discussion_r1056654063


##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PrimitivesTextNumber1.scala:
##########
@@ -78,18 +121,18 @@ case class ConvertTextNumberParser(
     // will match either all or none of 'str', never part of it. Thus,
     // findFirstIn() either matches and it's a zero rep, or it doesn't and it's
     // not a zero
-    val numValue = zeroRepsRegex.find { _.findFirstIn(str).isDefined } match {
+    val numValue: DataValueNumber = zeroRepsRegex.find { _.findFirstIn(str).isDefined } match {

Review Comment:
   Test interactions with zero reps, and for 'standard' with float/double with NaN and infinity reps. 



##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/parsers/PrimitivesTextNumber1.scala:
##########
@@ -54,11 +54,54 @@ case class ConvertTextCombinatorParser(
   }
 }
 
+trait TextDecimalVirtualPointMixin {
+  def textDecimalVirtualPoint: Int
+
+  final protected val divisor = scala.math.pow(10.0, textDecimalVirtualPoint)
+  final protected val multiplier = scala.math.pow(10.0, textDecimalVirtualPoint)
+
+  final protected def applyTextDecimalVirtualPointForParse(num1: Number): Number = {
+    if (textDecimalVirtualPoint == 0) num1
+    else {
+      // scale for virtual decimal point
+      val scaledNum: Number = num1 match {
+        case l: java.lang.Long => java.math.BigDecimal.valueOf(l).scaleByPowerOfTen(-textDecimalVirtualPoint)

Review Comment:
   Note asymmetry around type Long with unparser. 



##########
daffodil-lib/src/main/scala/org/apache/daffodil/util/DecimalUtils.scala:
##########
@@ -479,7 +479,10 @@ object DecimalUtils {
     decodedValue
   }
 
-  def zonedFromNumber(num: String, zonedStyle: TextZonedSignStyle, opl: OverpunchLocation.Value): String = {
+  def zonedFromNumber(

Review Comment:
   Revert this change. 



##########
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesTextNumber.scala:
##########
@@ -46,8 +46,231 @@ case class ConvertTextCombinator(e: ElementBase, value: Gram, converter: Gram)
   override lazy val unparser = new ConvertTextCombinatorUnparser(e.termRuntimeData, value.unparser, converter.unparser)
 }
 
-case class ConvertTextNumberPrim(e: ElementBase)
-  extends Terminal(e, true) {
+// This is a separate object for unit testing purposes.
+private[primitives]
+object TextNumberPatternUtils {
+
+  /**
+   * A regex which matches textNumberPatterns that legally use the V
+   * (implied decimal point) character.
+   */
+   private[primitives] lazy val vregexStandard = {
+    //  DFDL v1.0 Spec says
+    //  It is a Schema Definition Error if any symbols other than "0", "1" through "9" or #
+    //  are used in the vpinteger region of the pattern.
+    //
+    // The prefix and suffix chars can surround the vpinteger region, and there can be
+    // a positive and a negative pattern.
+    //
+    // The prefix and suffix cannot be digits, # or P or V
+    //
+    val prefix = """([^0-9#PV]?)"""
+    val suffix = prefix // same syntax as prefix
+    val vpinteger = """(#*[0-9]+)V([0-9]+)"""
+    val subpattern = s"""${prefix}${vpinteger}${suffix}"""
+     // 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"""^${subpattern}(?:;${subpattern})?$$"""
+    val re = vPattern.r
+    re
+  }
+
+  def textDecimalVirtualPointForStandard(
+    e: ImplementsThrowsOrSavesSDE,
+    originalPattern: String,
+    patternStripped: String): Int = {
+    val r = TextNumberPatternUtils.vregexStandard
+    r.findFirstMatchIn(patternStripped) match {
+      case Some(r(_, _, afterV, _, _, _, _, _)) => afterV.length
+      case None =>

Review Comment:
   Maybe provide better breakdown of syntax to provide better diagnostics here. 
   This may help consolidate all the ad-hoc checking.



##########
daffodil-core/src/main/scala/org/apache/daffodil/grammar/primitives/PrimitivesTextNumber.scala:
##########
@@ -46,8 +46,231 @@ case class ConvertTextCombinator(e: ElementBase, value: Gram, converter: Gram)
   override lazy val unparser = new ConvertTextCombinatorUnparser(e.termRuntimeData, value.unparser, converter.unparser)
 }
 
-case class ConvertTextNumberPrim(e: ElementBase)
-  extends Terminal(e, true) {
+// This is a separate object for unit testing purposes.
+private[primitives]
+object TextNumberPatternUtils {
+
+  /**
+   * A regex which matches textNumberPatterns that legally use the V
+   * (implied decimal point) character.
+   */
+   private[primitives] lazy val vregexStandard = {
+    //  DFDL v1.0 Spec says
+    //  It is a Schema Definition Error if any symbols other than "0", "1" through "9" or #
+    //  are used in the vpinteger region of the pattern.
+    //
+    // The prefix and suffix chars can surround the vpinteger region, and there can be
+    // a positive and a negative pattern.
+    //
+    // The prefix and suffix cannot be digits, # or P or V
+    //
+    val prefix = """([^0-9#PV]?)"""
+    val suffix = prefix // same syntax as prefix
+    val vpinteger = """(#*[0-9]+)V([0-9]+)"""
+    val subpattern = s"""${prefix}${vpinteger}${suffix}"""
+     // 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"""^${subpattern}(?:;${subpattern})?$$"""
+    val re = vPattern.r
+    re
+  }
+
+  def textDecimalVirtualPointForStandard(
+    e: ImplementsThrowsOrSavesSDE,
+    originalPattern: String,
+    patternStripped: String): 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 number pattern is mandatory.""".stripMargin('|'),
+          originalPattern)
+    }
+  }
+
+  private[primitives] lazy val vregexZoned= {
+    // Note: for zoned, can only have a positive pattern
+    // Prefix or suffix can only be '+' character, to
+    // indicate leading or trailing sign, and can only
+    // one of those.
+    //
+    // Also we're not allowing the # character, since I think that
+    // makes no sense for zoned with virtual decimal point.
+    //
+    val vPattern = """^(\+?)([0-9]+)V([0-9]+)(\+?)$""" // only positive pattern allowed
+    val re = vPattern.r
+    re
+  }
+
+  /**
+   * Checks a pattern for suitability with V (virtual decimal point) in the pattern
+   * in the context of zoned textNumberRep.
+   *
+   * This is broken out separately for unit testing purposes.
+   *
+   * @param pattern the dfdl:textNumberPattern pattern
+   * @return -1 if the pattern is illegal syntax for use with V (virtual decimal point)
+   *         otherwise the number of digits to the right of the V character.
+   */
+  private[primitives] def textDecimalVirtualPointForZoned(pattern: String): Option[Int] = {
+    val r = TextNumberPatternUtils.vregexZoned
+    r.findFirstMatchIn(pattern) match {
+      // note: cannot have both a prefix and suffix. Only one of them.
+      case Some(r(pre, _, afterV, suf)) if (pre.length + suf.length <= 1) => Some(afterV.length)

Review Comment:
   Better diagnostic behavior here. 



-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org