-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds support for writing Ion 1.1 sexps and structs #665
Adds support for writing Ion 1.1 sexps and structs #665
Conversation
@@ -236,7 +236,7 @@ private IonTypeID(byte id, int minorVersion) { | |||
(upperNibble == 0xD && lowerNibble >= 0x2) | |||
|| id == DELIMITED_STRUCT | |||
|| id == VARIABLE_LENGTH_INLINE_SYMBOL | |||
|| id == VARIABLE_LENGTH_STRUCT_WITH_FLEXSYMS | |||
|| id == VARIABLE_LENGTH_STRUCT_WITH_FLEX_SYMS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This got renamed so that the FLEX_SYMS
is consistent with FlexSyms
.
*/ | ||
fun stepInStruct(delimited: Boolean, useFlexSym: Boolean) | ||
fun stepInStruct(delimited: Boolean) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're automatically switching to FlexSyms when needed, this method parameter isn't really needed (for now) it can always be added back again later.
/** | ||
* A byte representing zero, encoded as a FlexInt (or FlexUInt). | ||
*/ | ||
const val ZERO: Byte = 0x01 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I introduced this because when we know that we're writing a FlexInt 0 (e.g. for a FlexSym), we may as well skip the conversion logic and write this byte directly.
/** | ||
* Clears this [ContainerInfo] of old data and initializes it with the given new data. | ||
*/ | ||
fun reset(type: ContainerType? = null, position: Long = -1, isDelimited: Boolean = false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The clear()
method was useful, but it set all the fields to the default values, and then we had to set them to the correct values. The new reset()
method skips some of the unnecessary field assignments.
* The string may contain multiple lines. Anything after a `|` character on a line is ignored, so | ||
* you can use `|` to add comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nice
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## ion-11-encoding #665 +/- ##
==================================================
Coverage ? 64.71%
Complexity ? 5722
==================================================
Files ? 199
Lines ? 24727
Branches ? 4393
==================================================
Hits ? 16001
Misses ? 7428
Partials ? 1298 ☔ View full report in Codecov by Sentry. |
Issue #, if available:
None
Description of changes:
Adds support for SExp and Structs in the
IonRawBinaryWriter_1_1
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.