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

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