feat: add registry-driven extension parsing and formatting#40
feat: add registry-driven extension parsing and formatting#40wackywendell merged 11 commits intomainfrom
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
wackywendell
force-pushed
the
extensions
branch
2 times, most recently
from
1f17b29 to
fe7a50d
Compare
wackywendell
force-pushed
the
extensions
branch
from
041da34 to
2b393dd
Compare
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
wackywendell
force-pushed
the
extensions
branch
from
2b393dd to
289763a
Compare
|
@codex review |
|
@cursor review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 062efe0d0b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Rule::expression => { | ||
| // TODO: Handle expressions properly when ExtensionColumn::Expression is implemented | ||
| unimplemented!("Expression handling in ExtensionColumn not yet implemented") | ||
| } |
There was a problem hiding this comment.
The grammar allows expression in extension_column (see src/parser/expression_grammar.pest), but this branch calls unimplemented!, so a valid plan containing an expression output column will panic the parser instead of returning a ParseError. This is a runtime crash path for user-supplied input; the branch should return a structured parse error (or support an expression column variant) rather than aborting.
Useful? React with 👍 / 👎.
Adds support for custom extension relations in the parser and textify system. Includes extension registry, Any encoding/decoding, and conversion utilities.
Enhances linting, adds test coverage, and completes extension relation refactoring with improved textify support.
…literals Address PR #40 review comments: - Replace unimplemented!() panic in ExtensionColumn expression parsing with Expression(RawExpression) variant - Replace silent string coercion of expression arguments with proper Expression variant (no more data loss on round-trip) - Escape string literals in ExtensionValue::Textify using escaped() - Remove dead From<&ExtensionValue> for Value impl
wackywendell
force-pushed
the
extensions
branch
from
062efe0 to
b9d1142
Compare
Centralizes relation parsing context, cleans clippy warnings, and simplifies the Parser API to enforce hard errors for extension parsing.
Moves name() into Explainable trait, simplifies Any/AnyConvertible, cleans up extension errors, and improves examples with ErrorList usage.
Simplifies trait bounds and Any implementation, removes debug prints, simplifies main.rs, enables cli feature by default, and cleans up extension naming.
Introduces ArgsExtractor for easier argument validation and simplifies the extension args API with expect_named_arg and get_named_or helpers.
Add comprehensive documentation for the custom extension type system: - Document the three requirements: prost::Message, prost::Name, Explainable - Document the Explainable trait and ArgsExtractor helpers - Add a compilable code example using the ExtensionRegistry - Reference examples/extensions.rs for complete implementation - Update GRAMMAR.md with Extension Relations section
…literals Address PR #40 review comments: - Replace unimplemented!() panic in ExtensionColumn expression parsing with Expression(RawExpression) variant - Replace silent string coercion of expression arguments with proper Expression variant (no more data loss on round-trip) - Escape string literals in ExtensionValue::Textify using escaped() - Remove dead From<&ExtensionValue> for Value impl
wackywendell
force-pushed
the
extensions
branch
from
b9d1142 to
4988988
Compare
2 participants
Description
ExtensionArgs,ExtensionValue,Any) so parser and formatter can translate extension relations to typed protobuf payloads.ParseError::ExtensionDetailfor registry failures.Type of Change
Testing
Checklist
Related Issues
Closes #32