You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "mbeckerle (via GitHub)" <gi...@apache.org> on 2023/04/18 20:50:44 UTC

[GitHub] [daffodil] mbeckerle opened a new pull request, #1006: Fix textStandardZeroRep to be "" not "0"

mbeckerle opened a new pull request, #1006:
URL: https://github.com/apache/daffodil/pull/1006

   This causes trouble when people use textNumberPattern expecting say, 000, but get just 0.
   
   It is simply wrong for this property to be anything by default but ""; however, this change does change what happens when standard text numbers are unparsed that are zero valued. 
   
   This change may cause backward compatibility issues, and we need to regression test against all schemas, which is why I want to get this into release 3.5.0, and not wait until after.
   
   If the compatibility issues are significant enough, we can revert this change, or build in a switch, but I want to try it first.
   
   DAFFODIL-2147


-- 
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


[GitHub] [daffodil] mbeckerle merged pull request #1006: Fix textStandardZeroRep to be "" not "0"

Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle merged PR #1006:
URL: https://github.com/apache/daffodil/pull/1006


-- 
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


[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #1006: Fix textStandardZeroRep to be "" not "0"

Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1006:
URL: https://github.com/apache/daffodil/pull/1006#discussion_r1170574502


##########
daffodil-lib/src/main/resources/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd:
##########
@@ -92,7 +92,7 @@
           textStandardGroupingSeparator=","
           textStandardInfinityRep="Inf"
           textStandardNaNRep="NaN"
-          textStandardZeroRep="0"
+          textStandardZeroRep=""

Review Comment:
   Just so I'm clear, by setting this to the empty string does it mean that there is no zero text representation and so infosets that have a value of zero just unparse using the `dfdl:textNumberPattern`? Or does it mean that values of zero in the infoset unparse to the empty string? I assume the former?
   
   It might be helpful to add that to the commit message, maybe as a `Deprecation/Compatibility` section so that we remember to add a blurb about this change in the release notes, since this could break some schemas.
   
   If we don't have one already, it might also be a good idea to add a test that makes sure a zero value actually unparses using the pattern and doesn't unparse to the empty string or something weird. It looks like `tnp91` in `TextNumberProps.tdml` is the only test that explicitly sets `textStandardRep` to the empty string, but the only test that uses it is a negative test and doesn't show what a zero value would actually unparse to.



-- 
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


[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1006: Fix textStandardZeroRep to be "" not "0"

Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1006:
URL: https://github.com/apache/daffodil/pull/1006#discussion_r1171273827


##########
daffodil-lib/src/main/resources/org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd:
##########
@@ -92,7 +92,7 @@
           textStandardGroupingSeparator=","
           textStandardInfinityRep="Inf"
           textStandardNaNRep="NaN"
-          textStandardZeroRep="0"
+          textStandardZeroRep=""

Review Comment:
   Great suggestions. I'll add a test that insures this is working properly. you are correct that empty string turns "off" the feature of special zero rep. 



-- 
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