Add expressionAllowed param to getPropertyOption#1312
Conversation
stevedlawrence
left a comment
stevedlawrence
left a comment
There was a problem hiding this comment.
+1 👍 just minor suggestions for possible cleanup
| "textStandardInfinityRep", | ||
| "textStandardNaNRep", | ||
| "textStandardZeroRep", | ||
| "nilValue", | ||
| "textStringPadCharacter", | ||
| "textNumberPadCharacter", | ||
| "textBooleanPadCharacter", |
There was a problem hiding this 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
| .replaceAll("false", "true") // add expression allowed for these | ||
| } else { | ||
| template.replaceAll("currency", propName) | ||
| } |
There was a problem hiding this comment.
Thoughts on changing false to expressionAllowed and then just always do something like this?
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.
| lazy val textStandardExponentRep: Found = { | ||
| val tsec = getPropertyOption("textStandardExponentCharacter") | ||
| val tser = getPropertyOption("textStandardExponentRep") | ||
| val tser = getPropertyOption("textStandardExponentRep", expressionAllowed = true) |
There was a problem hiding this comment.
The bug mentioned several properties that needed this. Did you confirm this is the only case that's missing?
There was a problem hiding this comment.
Yes, most of the properties that needed it were called by GeneratedCode.scala...this was the only one that needed it that was called outside generated code
- move non-runtimed valued properties to excludedBecauseDoneByHand - update function calls - update tests to ensure we no longer get warnings about textStandardExponentRep DAFFODIL-2928
olabusayoT
force-pushed
the
daf-2928-fixGetPropertyOptionCalls
branch
from
93f7afb to
b5be895
Compare
3 participants
DAFFODIL-2928