Adds support for parsing Ion 1.1 encoding directives in the text reader. #954
Conversation
| @@ -25,6 +29,18 @@ class IonRawTextWriter_1_1 internal constructor( | |||
|
|
|||
| companion object { | |||
| const val IVM = "\$ion_1_1" | |||
|
|
|||
| @JvmStatic | |||
| fun from(output: OutputStream, blockSize: Int, options: IonTextWriterBuilder_1_1): IonRawTextWriter_1_1 { | |||
There was a problem hiding this comment.
This mirrors a utility I added on the binary side for testing.
| resetSymbolTable(); // TODO handle appended symbols | ||
| if (!isSymbolTableAppend) { |
There was a problem hiding this comment.
This is a case where writing the code to do what the TODO said to do would have been faster than writing the TODO itself.
| @@ -1361,10 +1363,6 @@ void readEncodingDirective() { | |||
| newMacros.put(MacroRef.byId(++localMacroMaxOffset), newMacro); | |||
| state = State.IN_MACRO_TABLE_SEXP; | |||
| break; | |||
| case COMPILING_MACRO: | |||
There was a problem hiding this comment.
I found that this state is unreachable due to a check elsewhere in the reader.
| protected MacroEvaluatorAsIonReader macroEvaluatorIonReader = null; | ||
|
|
||
| // The core MacroEvaluator that this core reader delegates to when evaluating a macro invocation. | ||
| private MacroEvaluator macroEvaluator = null; | ||
|
|
||
| // The encoding context (macro table) that is currently active. | ||
| private EncodingContext encodingContext = null; |
There was a problem hiding this comment.
These will start to be used in the future PR that adds macro evaluation.
| * found along the way. | ||
| * @return true if the reader is positioned on a value; otherwise, false. | ||
| */ | ||
| protected final boolean has_next_system_value() { |
There was a problem hiding this comment.
This mimics the existing naming convention for the has_next methods. Previously, no system-level has_next was needed because the system-level reader did not interpret any system values. However, as in the binary reader, macro compilation and evaluation has to happen one layer below Ion 1.0 system table handling since Ion 1.1 macros can expand to system values.
| @@ -0,0 +1,145 @@ | |||
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
| package com.amazon.ion.impl.macro | |||
There was a problem hiding this comment.
This should probably be at com.amazon.ion.impl.
| import com.amazon.ion.impl.macro.MacroRef.Companion.byId | ||
|
|
||
| /** | ||
| * Reads encoding directives from the given [IonReader]. |
There was a problem hiding this comment.
Can you add a comment about the potential reusability of this class? IIRC, there's also an encoding directive reader in IonReaderContinuableCore that is only compatible with that class. Is this one actually usable with multiple readers, and if so, which ones?
There was a problem hiding this comment.
I'll add a comment. There is one in IonReaderContinuableCore that is more complicated because it has to deal with continuability. I ultimately decided to pare this one down to only what's necessary for IonReader, rather than try to unify them. There isn't that much code here, and most of the complexity is in the MacroCompiler, which has been generalized for both readers.
tgregg
force-pushed
the
ion-11-encoding-text-read-encoding-directive
branch
from
6b84798
to
3260beb
Compare
Description of changes:
Macro evaluation in Ion text will come in a future PR. At that point, all tests in
EncodingDirectiveCompilationTestwill be parameterized to exercise both binary and text.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.