stevedlawrence commented on code in PR #1312:
URL: https://github.com/apache/daffodil/pull/1312#discussion_r1775643897
##########
daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/PropertyGenerator.scala:
##########
@@ -430,41 +430,44 @@ trait CurrencyMixin extends PropertyMixin {
"""
+ private val expressionAllowedProperties = List(
+ "byteOrder",
+ "encoding",
+ "initiator",
+ "terminator",
+ "outputNewLine",
+ "length",
+ "escapeCharacter",
+ "escapeEscapeCharacter",
+ "textStandardDecimalSeparator",
+ "textStandardGroupingSeparator",
+ "textStandardExponentRep",
+ "binaryFloatRep",
+ "textBooleanTrueRep",
+ "textBooleanFalseRep",
+ "separator",
+ "occursCount",
+ "inputValueCalc",
+ "outputValueCalc",
+ "calendarLanguage",
+ "choiceDispatchKey"
+ )
+
/**
* exclude these since code should use the CompiledExpression created from
these values,
* not the property values themselves. We'll do these by hand.
*/
def excludeRuntimeProperties(propName: String) = {
val runtimeValuedProperties = List(
- "byteOrder",
- "encoding",
- "initiator",
- "terminator",
- "outputNewLine",
- "length",
- "escapeCharacter",
- "escapeEscapeCharacter",
- "textStandardDecimalSeparator",
- "textStandardGroupingSeparator",
- "textStandardExponentRep",
- "binaryFloatRep",
- "textBooleanTrueRep",
- "textBooleanFalseRep",
- "separator",
- "occursCount",
- "inputValueCalc",
- "outputValueCalc",
"textStandardInfinityRep",
"textStandardNaNRep",
"textStandardZeroRep",
"nilValue",
"textStringPadCharacter",
"textNumberPadCharacter",
"textBooleanPadCharacter",
Review Comment:
The ones now in the `runtimeValuedProperties` are not really runtime valued
properties, they are just done by hand for some reason (i'm not sure why?). I
wonder if they really belong in the `excludedBecauseDoneByHand` member up
above? And then `excludeRuntimeProperties` really only has to look at
`expressionAllowedProperities`
##########
daffodil-lib/src/main/scala/org/apache/daffodil/lib/schema/annotation/props/ByHandMixins.scala:
##########
@@ -451,7 +451,7 @@ trait TextStandardExponentRepMixin extends PropertyMixin {
lazy val textStandardExponentRep: Found = {
val tsec = getPropertyOption("textStandardExponentCharacter")
- val tser = getPropertyOption("textStandardExponentRep")
+ val tser = getPropertyOption("textStandardExponentRep", expressionAllowed
= true)
Review Comment:
The bug mentioned several properties that needed this. Did you confirm this
is the only case that's missing?
##########
daffodil-propgen/src/main/scala/org/apache/daffodil/propGen/PropertyGenerator.scala:
##########
@@ -642,12 +645,18 @@ object Currency {
*/
def generateNonEnumStringPropInit(propName: String) = {
val template =
- """registerToStringFunction(()=>{getPropertyOption("currency") match {
+ """registerToStringFunction(()=>{getPropertyOption("currency", false)
match {
case None => ""
case Some(value) => "currency='" + value.toString + "'"
}
})"""
- val res = template.replaceAll("currency", propName)
+ val res = if (expressionAllowedProperties.contains(propName)) {
+ template
+ .replaceAll("currency", propName)
+ .replaceAll("false", "true") // add expression allowed for these
+ } else {
+ template.replaceAll("currency", propName)
+ }
Review Comment:
Thoughts on changing `false` to `expressionAllowed` and then just always do
something like this?
```scala
val expressionAllowStr =
expressionAllowedProperties.contains(propName).toString
template
.replace("currency", propName)
.replace("expressionAllowed", expressionAllowedStr)
```
Avoids the replace duplicate code and matches on something less common to
avoid possible incorrect replacements if we ever tweak this template.
--
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]
