Remove unnecessary truncation code #1341
Conversation
olabusayoT
force-pushed
the
daf-1592-unparsingStringTruncation
branch
2 times, most recently
from
83893ee
to
6e52733
Compare
| // variable-width encodings and lengthUnits characters, and lengthKind explicit | ||
| // is not supported (currently) for complex types | ||
| // | ||
| state.schemaDefinitionUnless( |
There was a problem hiding this comment.
Just trying to remember, is it still an SDE if it is something that just isn't supported yet?
There was a problem hiding this comment.
Good point, I think we usually use subset/subsetError, which is an SDE but mentions it's a "subset".
|
|
||
| // truncation is done by StringMaybeTruncateCharactersUnparser | ||
| eUnparser.unparse1(state, erd) | ||
| } |
There was a problem hiding this comment.
It seems SpecifiedLengthExplicitImplicitUnparser doesn't really do anything anymore. I wonder if it can just be greatly simplified? The binary and fixed width text cases call unparseBits which just call eUnparser.unpase1(...) (it evaluates target length but I'm not sure if that's needed anymore?). And now the non-fixed width text case also calls eUnparser.unparse1(...), just with an SDE check for complex types.
Feels like all of the real work is now moved out of this unparser and into others. Can this just be simplified to
if (text representation AND lengthUnits characters AND non fixed width AND isComplex) {
SDE(...)
} else {
// the child unparser inspects target length and handles truncation and padding/fill
// nothing needs to be done in this unparser
eUnparser.unparse1(...)
}And literally everything else about targetLength and unparseBits goes away?
- we were truncating and overwriting value, but we didn't actually need to since we ended up truncating again right after. Removed the unnecessary and buggy code which changed the dataValue. StringMaybeTruncateBitsUnparser.unparse does not change the dataValue, instead it changes the DOS, resulting in the value getting truncated but string-length returning the untruncated length when it's called - move subset/subsetError to ThrowsSDE trait so state has access to it - refactor SpecifiedLengthExplicitImplicitUnparser to be more straightforward and to remove unused cruft - add tests DAFFODIL-1592
olabusayoT
force-pushed
the
daf-1592-unparsingStringTruncation
branch
from
a795012
to
6eb8158
Compare
DAFFODIL-1592