-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat(parse): add proto::extensions::SimpleExtensionDeclaration
parser
#170
base: main
Are you sure you want to change the base?
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.
Thank you for your contribution. I left some comments.
Marked this as non-breaking (for now).
src/parse/proto/extensions/simple_extension_declaration/extension_function.rs
Outdated
Show resolved
Hide resolved
src/parse/proto/extensions/simple_extension_declaration/extension_function.rs
Outdated
Show resolved
Hide resolved
type Parsed = ExtensionFunction; | ||
type Error = SimpleExtensionDeclarationError; | ||
|
||
fn parse(self, _ctx: &mut C) -> Result<Self::Parsed, Self::Error> { |
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 parser should add the function to the context, checking that the referenced extension uri is valid, the used anchor is unique, and that the definition contains a matching function with the given name in the referenced extension.
src/parse/proto/extensions/simple_extension_declaration/extension_function.rs
Outdated
Show resolved
Hide resolved
use crate::parse::{Anchor, Context, Parse}; | ||
use crate::proto; |
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.
Nit: can you merge those?
type Parsed = ExtensionTypeVariation; | ||
type Error = SimpleExtensionDeclarationError; | ||
|
||
fn parse(self, _ctx: &mut C) -> Result<Self::Parsed, Self::Error> { |
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.
Same here.
src/parse/proto/extensions/simple_extension_declaration/mapping_type.rs
Outdated
Show resolved
Hide resolved
src/parse/proto/extensions/simple_extension_declaration/simple_extension_declaration.rs
Outdated
Show resolved
Hide resolved
|
||
Ok(SimpleExtensionDeclaration { | ||
mapping_type: mapping_type | ||
.map(|mapping_type| mapping_type.parse(ctx).ok()) |
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.
We don't want this to silently error on parser errors.
proto::extensions::SimpleExtensionDeclaration
parser
…ion_function.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
…ion_function.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
…ion_function.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
…ion_type.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
…ion_type.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
…ion_type_variation.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
…ion_type_variation.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
…g_type.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
…_extension_declaration.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
src/parse/context.rs
Outdated
/// Add a [ExtensionFunction] to this context. Must return an error for duplicate | ||
/// anchors, when the URI is not supported or if the definition does not contain a matching function with the given name. | ||
/// | ||
/// This function must eagerly resolve and parse the simple extension, returning an | ||
/// error if either fails. | ||
fn add_extension_function( | ||
&mut self, | ||
extension_function: &ExtensionFunction, | ||
) -> Result<&SimpleExtensions, ContextError>; |
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 return types and comments of these functions should be updated.
src/parse/proto/extensions/simple_extension_declaration/simple_extension_declaration.rs
Outdated
Show resolved
Hide resolved
src/parse/proto/extensions/simple_extension_declaration/simple_extension_declaration.rs
Outdated
Show resolved
Hide resolved
…_extension_declaration.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
…_extension_declaration.rs Co-authored-by: Matthijs Brobbel <m1brobbel@gmail.com>
Adds a parser for proto::extensions::SimpleExtensionDeclaration that parses the declaration.