Adds support for Ion 1.1 to inspect
#139
Conversation
* Ion 1.1 value literals are recognized and their encodings displayed in the expected fashion. * Ion 1.1 macro invocations (e-expressions) at the top level have their encodings displayed; the ephemeral values they produce follow them in a dimmed font. * This implementation does not yet support displaying e-expressions nested inside containers.
| @@ -157,11 +157,6 @@ impl WithIonCliArgument for ClapCommand { | |||
| ) | |||
| } | |||
|
|
|||
| /// All commands automatically have the "unstable" opt-in flag. This makes it visible. | |||
There was a problem hiding this comment.
🗺️ This impl was moved so the method order would align with the trait's method order.
| @@ -18,6 +18,14 @@ use crate::output::CommandOutput; | |||
| pub struct InspectCommand; | |||
|
|
|||
| impl IonCliCommand for InspectCommand { | |||
| fn is_stable(&self) -> bool { | |||
There was a problem hiding this comment.
🗺️ These methods were moved to align their order with that of the trait.
| let item = reader.next_item()?; | ||
| let is_last_item = matches!(item, SystemStreamItem::EndOfStream(_)); | ||
| item.raw_stream_item() | ||
| .map(|i| self.confirm_encoding_is_supported(i.encoding())) | ||
| .unwrap_or(Ok(()))?; | ||
|
|
There was a problem hiding this comment.
🗺️ Prior to version rc.7, ion-rust didn't have a way to get the next encoded item in the stream AND the values to which it expanded. You could only get one or the other. This meant that you couldn't display the encoding of an e-expression because the reader would only surface its expanded values. The code snippet that follows uses reader.next_expanded_item(), which returns an enum of either values (literal or ephemeral) or e-expressions.
| match self.select_action( | ||
| TOP_LEVEL_DEPTH, | ||
| &mut has_printed_skip_message, | ||
| &item.raw_stream_item(), | ||
| &maybe_raw_item, |
There was a problem hiding this comment.
🗺️ If this stream item isn't ephemeral, see if it's in the requested range of bytes to inspect.
| SystemStreamItem::SymbolTable(lazy_struct) => { | ||
| match expr { | ||
| EExp(eexp) => { | ||
| self.inspect_eexp(0, eexp)?; |
There was a problem hiding this comment.
🗺️ We now display e-expressions found at the top level. Some more work is needed in ion-rust to make it possible to get an 'expanded' view of lists and structs, allowing us to inspect both the encoded expressions and the values/fields they produce.
| self.confirm_encoding_is_supported(marker.encoding())?; | ||
| self.inspect_ivm(marker)?; | ||
| } | ||
| SystemStreamItem::EndOfStream(_) => { | ||
| EndOfStream(end) => { | ||
| self.inspect_end_of_stream(end.range().start)?; |
There was a problem hiding this comment.
🗺️ The end of the stream now gets a row with an // End of stream comment and (more importantly) the offset where the end occurs.
| vec![ | ||
| // TODO: Add methods to EExpression that allow nuanced access to its encoded spans | ||
| // TODO: Length-prefixed and multibyte e-expression addresses | ||
| IonBytes::new(BytesKind::MacroId, &eexp.span().bytes()[0..1]), | ||
| ], |
There was a problem hiding this comment.
🗺️ For now we assume that the e-expression's ID is just the first byte of the encoding. We need to add a method here to get the correct byte slice more reliably.
| Binary_1_0(bin_val) => { | ||
| self.inspect_literal_scalar(depth, delimiter, value, bin_val, comment_fn) | ||
| } | ||
| Binary_1_1(bin_val) => { | ||
| self.inspect_literal_scalar(depth, delimiter, value, bin_val, comment_fn) |
There was a problem hiding this comment.
🗺️ The inspect_literal_scalar method is generic over the encoding of the binary value, so these two method calls look identical but they're different.
There was a problem hiding this comment.
Is that worth a code comment, or is it obvious at a glance to people more experienced with Rust and/or this code base than me?
There was a problem hiding this comment.
It's more obvious in an IDE, but I though it might raise an eyebrow in the context of the diff.
zslayton
changed the title
inspect
| Binary_1_0(bin_val) => { | ||
| self.inspect_literal_scalar(depth, delimiter, value, bin_val, comment_fn) | ||
| } | ||
| Binary_1_1(bin_val) => { | ||
| self.inspect_literal_scalar(depth, delimiter, value, bin_val, comment_fn) |
There was a problem hiding this comment.
Is that worth a code comment, or is it obvious at a glance to people more experienced with Rust and/or this code base than me?
ion-cli's dependency onion-rstov1.0.0-rc.7.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.