update fixes vol 2 #3
Conversation
WalkthroughThe overarching change involves transitioning from the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (5)
Cargo.tomlis excluded by:!**/*.tomlderive/Cargo.tomlis excluded by:!**/*.tomlgenerator/Cargo.tomlis excluded by:!**/*.tomlmeta/Cargo.tomlis excluded by:!**/*.tomlpest/Cargo.tomlis excluded by:!**/*.toml
Files selected for processing (14)
- derive/examples/reader.rs (1 hunks)
- derive/src/lib.rs (1 hunks)
- derive/tests/grammar.rs (1 hunks)
- derive/tests/json.rs (2 hunks)
- derive/tests/sample.rs (1 hunks)
- derive/tests/sequence.rs (2 hunks)
- derive/tests/skip.rs (1 hunks)
- generator/src/common.rs (1 hunks)
- generator/src/typed/generator.rs (2 hunks)
- generator/src/typed/module.rs (2 hunks)
- generator/src/types.rs (1 hunks)
- generator/tests/expected.rs (37 hunks)
- generator/tests/generator.rs (1 hunks)
- pest/tests/predefined_node.rs (1 hunks)
Files not summarized due to errors (1)
- generator/tests/expected.rs: Error: Message exceeds token limit
Additional comments: 20
generator/src/types.rs (1)
- 6-6: The update from
::pestto::pest3in thepest()function aligns with the PR's objectives and appears correctly implemented.derive/src/lib.rs (1)
- 16-16: The update from
pest_generatortopest3_generatorin thederive_typed_parserfunction is correctly implemented and aligns with the PR's objectives.derive/examples/reader.rs (1)
- 1-2: The updates to imports from
pesttopest3andpest_derivetopest3_deriveare correctly implemented and align with the PR's objectives.derive/tests/sample.rs (1)
- 3-4: The updates to imports from
pesttopest3forTypedNodeandParserare correctly implemented and align with the PR's objectives.generator/src/common.rs (3)
- 3-3: The update of the import statement from
pest_metatopest3_metaaligns with the PR objectives to transition to newer library versions. This change looks good.- 1-6: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [6-20]
The
generate_includefunction correctly generates Rustinclude_str!for grammar files, making paths relative to the current directory. This approach enhances portability and maintainability.
- 1-6: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [22-42]
The
generate_rule_enumfunction effectively generates an enum for parser rules, with appropriate handling of documentation comments and exclusion of special rules. The use of thequote!macro for code generation is correctly applied.pest/tests/predefined_node.rs (2)
- 9-9: The update of the import statement from
pesttopest3aligns with the PR objectives to transition to newer library versions. This change looks good.- 6-12: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [11-117]
The test implementations in
predefined_node.rsare well-structured and cover a variety of cases to verify the functionality of predefined nodes using thepest3library. The tests appear to be comprehensive and correctly implemented.generator/src/typed/module.rs (2)
- 5-6: The update of the import statements from
pestandpest_metatopest3andpest3_metaaligns with the PR objectives to transition to newer library versions. These changes look good.- 235-235: The temporary change to make "pest" paths refer to the
pest3crate is a reasonable workaround to ensure compatibility during the transition period. This approach facilitates a smoother update process.derive/tests/grammar.rs (2)
- 14-18: The update of the import statements from
pesttopest3and frompest_derivetopest3_derivealigns with the PR objectives to transition to newer library versions. These changes look good.- 11-21: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [20-454]
The test implementations in
derive/tests/grammar.rsare well-structured and cover a wide range of parsing scenarios to verify the functionality of grammar parsing using thepest3library. The tests appear to be comprehensive and correctly implemented.generator/src/typed/generator.rs (2)
- 13-13: The update from
pest_metatopest3_metain the import statements aligns with the PR objectives to transition to the newer library versions. This change is crucial for ensuring compatibility with the updatedpest3ecosystem.- 10-16: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [590-590]
The use of
unwrapinderive_typed_parserfor parsing the input token stream is risky and could lead to panics if parsing fails. Consider handling this error more gracefully to provide clearer error messages or to allow for error propagation, enhancing the robustness of the code.generator/tests/expected.rs (5)
- 42-42: The implementation of
::pest3::typed::RuleType for Rulecorrectly updates the library usage topest3. However, it's essential to ensure that all references toRulethroughout the codebase are updated to reflect any changes in the enum's variants or their usage.- 57-57: The implementation for
::pest3::typed::wrapper::String for W0correctly specifies the content as a single space character. This is a good use of thepest3library's typed wrapper functionality. Ensure that all such wrappers are consistently used across the codebase for similar purposes.- 129-129: The struct
r#Regularcorrectly defines a rule with its content and span. It's important to ensure that the grammar specified in the documentation comment matches the actual implementation and usage of this rule in the parsing logic.- 1022-1022: The
r#Pushrule demonstrates an advanced use of thepest3library's stack manipulation features. It's essential to verify that the stack operations (push,pop,pop_all) are used correctly and that their interactions are well-understood and tested, especially in complex sequences.- 1506-1511: The
genericsmodule re-exports several utilities frompest3. This is a good practice for simplifying imports elsewhere in the code. However, ensure that these re-exports are used consistently throughout the project to avoid confusion.
| @@ -232,6 +232,7 @@ impl<'g> ModuleSystem<'g> { | |||
| ("stack", Rc::new(ModuleNode::Collection(pest_stack))), | |||
| ("unicode", Rc::new(pest_unicode)), | |||
| ]); | |||
| // FIXME: make "pest" paths to refer to pest3 crate for the moment | |||
There was a problem hiding this comment.
@TheVeryDarkness I tried to quickly rename the crates to pest3*, so that the prototype can later be published on crates.io independently of existing pest crates, but it managed to break generator/derive due to the intermingling with "pest" path imports in the grammar (to be clear, the paths in the grammars should still be "pest::", not "pest3").
Anyway, as you were previously asking about the modules or working on them #2 (comment) , feel free to revert this change or refactor it as you see fit
There was a problem hiding this comment.
I see, and that's fine. I've been working on that for days but haven't made any commit, so it won't affect me.
Summary by CodeRabbit
pesttopest3across various files, enhancing token handling, parser derivation, and parsing logic.