-
Notifications
You must be signed in to change notification settings - Fork 36
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
Version bump to 1.0.0-rc.6, improvements to IonEncoding #785
Conversation
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.
🗺️ PR tour 🧭
Text_1_0 | Binary_1_0 => (1, 0), | ||
Text_1_1 | Binary_1_1 => (1, 1), | ||
} | ||
} |
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.
🗺️ These methods make it easier for programs like ion inspect
to check what encoding they're working with and construct error messages.
@@ -213,6 +213,7 @@ macro_rules! v1_x_reader_writer { | |||
lazy::r#struct::{LazyStruct, LazyField}, | |||
lazy::sequence::{LazyList, LazySExp}, | |||
lazy::encoder::value_writer::{ValueWriter, StructWriter, SequenceWriter, EExpWriter}, | |||
lazy::any_encoding::IonEncoding, |
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 re-export is only visible when the experimental-reader-writer
feature is enabled.
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.
🗺️ PR tour part 2 🧭
Binary_1_1(_) => BinaryEncoding_1_1.encoding(), | ||
} | ||
} | ||
} |
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 plumbed the IonEncoding
from the reader up through all of the raw stream item types (version marker, value, e-expression, end of stream), making it easy to check the encoding of any item during iteration.
position: usize, | ||
} | ||
|
||
impl EndPosition { | ||
pub(crate) fn new(position: usize) -> Self { | ||
Self { position } | ||
pub(crate) fn new(encoding: IonEncoding, position: usize) -> Self { |
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.
🗺️ While it seems a bit silly for the end of the stream to have an 'encoding' despite being a zero-length entity, this enables the LazyRawStreamItem<AnyEncoding>
to always report its encoding.
/// a symbol table. | ||
pub fn symbol_table(self) -> Option<LazyStruct<'top, D>> { | ||
/// If this item is a symbol table, returns `Some(lazy_struct)`. Otherwise, returns `None`. | ||
pub fn as_symbol_table(self) -> Option<LazyStruct<'top, D>> { |
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 renamed this method for consistency; other types use as_something
/expect_something
for their Option<T>
/IonResult<T>
pairs.
src/lazy/value.rs
Outdated
@@ -66,9 +66,17 @@ impl<'top, D: Decoder> LazyValue<'top, D> { | |||
LazyValue { expanded_value } | |||
} | |||
|
|||
fn symbol_table(&'top self) -> &'top SymbolTable { | |||
#[cfg(feature = "experimental-tooling-apis")] | |||
pub fn symbol_table(&'top self) -> &'top SymbolTable { |
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.
🗺️ When the experimental-tooling-access
feature is enabled, values can allow access to their symbol table.
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.
LGTM 🚀
Changes:
v1.0.0-rc.6
IonEncoding
typeRawStreamItem
s to report theirIonEncoding
LazyValue
'sSymbolTable
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.