feat: Context trait
#66
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Warning Rate Limit Exceeded@arendjr has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 52 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe project underwent a structural refinement, introducing a new 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: 1
Configuration used: CodeRabbit UI
Files selected for processing (64)
- crates/core/src/context.rs (1 hunks)
- crates/core/src/lib.rs (1 hunks)
- crates/core/src/pattern.rs (7 hunks)
- crates/core/src/pattern/accessor.rs (3 hunks)
- crates/core/src/pattern/accumulate.rs (6 hunks)
- crates/core/src/pattern/add.rs (4 hunks)
- crates/core/src/pattern/after.rs (3 hunks)
- crates/core/src/pattern/and.rs (3 hunks)
- crates/core/src/pattern/any.rs (3 hunks)
- crates/core/src/pattern/assignment.rs (3 hunks)
- crates/core/src/pattern/ast_node.rs (4 hunks)
- crates/core/src/pattern/before.rs (3 hunks)
- crates/core/src/pattern/boolean_constant.rs (2 hunks)
- crates/core/src/pattern/bubble.rs (2 hunks)
- crates/core/src/pattern/built_in_functions.rs (17 hunks)
- crates/core/src/pattern/call.rs (4 hunks)
- crates/core/src/pattern/code_snippet.rs (2 hunks)
- crates/core/src/pattern/contains.rs (3 hunks)
- crates/core/src/pattern/divide.rs (4 hunks)
- crates/core/src/pattern/dynamic_snippet.rs (4 hunks)
- crates/core/src/pattern/equal.rs (2 hunks)
- crates/core/src/pattern/every.rs (2 hunks)
- crates/core/src/pattern/file_pattern.rs (2 hunks)
- crates/core/src/pattern/files.rs (2 hunks)
- crates/core/src/pattern/float_constant.rs (2 hunks)
- crates/core/src/pattern/function_definition.rs (5 hunks)
- crates/core/src/pattern/functions.rs (5 hunks)
- crates/core/src/pattern/if.rs (3 hunks)
- crates/core/src/pattern/includes.rs (2 hunks)
- crates/core/src/pattern/int_constant.rs (2 hunks)
- crates/core/src/pattern/like.rs (3 hunks)
- crates/core/src/pattern/limit.rs (2 hunks)
- crates/core/src/pattern/list.rs (3 hunks)
- crates/core/src/pattern/list_index.rs (3 hunks)
- crates/core/src/pattern/log.rs (4 hunks)
- crates/core/src/pattern/map.rs (2 hunks)
- crates/core/src/pattern/match.rs (2 hunks)
- crates/core/src/pattern/maybe.rs (4 hunks)
- crates/core/src/pattern/modulo.rs (4 hunks)
- crates/core/src/pattern/multiply.rs (4 hunks)
- crates/core/src/pattern/not.rs (3 hunks)
- crates/core/src/pattern/or.rs (3 hunks)
- crates/core/src/pattern/pattern_definition.rs (2 hunks)
- crates/core/src/pattern/patterns.rs (5 hunks)
- crates/core/src/pattern/predicate_definition.rs (2 hunks)
- crates/core/src/pattern/predicate_return.rs (2 hunks)
- crates/core/src/pattern/predicates.rs (2 hunks)
- crates/core/src/pattern/range.rs (2 hunks)
- crates/core/src/pattern/regex.rs (2 hunks)
- crates/core/src/pattern/resolved_pattern.rs (8 hunks)
- crates/core/src/pattern/rewrite.rs (4 hunks)
- crates/core/src/pattern/sequential.rs (3 hunks)
- crates/core/src/pattern/some.rs (2 hunks)
- crates/core/src/pattern/state.rs (1 hunks)
- crates/core/src/pattern/step.rs (6 hunks)
- crates/core/src/pattern/string_constant.rs (3 hunks)
- crates/core/src/pattern/subtract.rs (4 hunks)
- crates/core/src/pattern/test.rs (1 hunks)
- crates/core/src/pattern/undefined.rs (2 hunks)
- crates/core/src/pattern/variable.rs (2 hunks)
- crates/core/src/pattern/where.rs (2 hunks)
- crates/core/src/pattern/within.rs (2 hunks)
- crates/core/src/suppress.rs (3 hunks)
- crates/core/src/text_unparser.rs (1 hunks)
Additional comments: 126
crates/core/src/pattern/test.rs (2)
- 1-3: The introduction of the
Contexttrait is a significant architectural change. It abstracts context handling, allowing for more flexible implementations. Ensure that all components that previously relied on the concreteContextstruct are updated to work with this trait.- 5-9: The
BuiltInFunctionstruct and theFunctype alias demonstrate how theContexttrait can be utilized in function signatures. This is a good example of leveraging trait objects for greater flexibility. However, consider adding documentation comments to explain the purpose and usage of these constructs, especially since they play a crucial role in the new architecture.crates/core/src/pattern/undefined.rs (1)
- 15-15: The modification of the
_contextparameter to use&'a impl Context<'a>instead of a concrete type is in line with the PR's goal of abstracting context handling. This change enhances flexibility and modularity. Ensure that all usages of theUndefinedstruct'sexecutemethod are updated accordingly.crates/core/src/lib.rs (1)
- 4-4: The addition of the
contextmodule is a key part of the PR's architectural changes. It centralizes the definition and implementation of the newContexttrait, facilitating easier maintenance and future enhancements. Ensure that the module is properly documented to aid in understanding its role within the system.crates/core/src/pattern/int_constant.rs (1)
- 39-39: Adjusting the
_contextparameter to use&'a impl Context<'a>in theMatchertrait implementation forIntConstantaligns with the PR's architectural changes. This enhances flexibility in context handling. Ensure consistency in this approach across allMatcherimplementations.crates/core/src/pattern/float_constant.rs (1)
- 39-39: The adjustment of the
_contextparameter to use&'a impl Context<'a>in theMatchertrait implementation forFloatConstantis consistent with the PR's architectural changes. This approach enhances modularity and should be consistently applied across allMatcherimplementations.crates/core/src/pattern/files.rs (1)
- 27-27: The use of
&'a impl Context<'a>in theMatchertrait implementation forFilesis a good example of the PR's efforts to abstract context handling. This change promotes flexibility and should be consistently applied across all relevant implementations.crates/core/src/pattern/file_pattern.rs (1)
- 27-27: The adjustment of the
contextparameter to use&'a impl Context<'a>in theMatchertrait implementation forFilePatternaligns with the PR's goal of abstracting context handling. This change enhances modularity and should be consistently applied across allMatcherimplementations.crates/core/src/pattern/predicate_return.rs (2)
- 9-9: The import of the
Contexttrait aligns with the PR's objectives of introducing a newContexttrait for more flexible context handling.- 55-55: The update to the
execute_funcmethod signature, using a trait object for thecontextparameter, enhances flexibility and modularity in context handling. This change aligns with the PR's objectives.crates/core/src/pattern/includes.rs (1)
- 63-63: The modification of the
contextparameter type in theexecutemethod to use a trait object (&'a impl Context<'a>) aligns with the PR's goal of abstracting context handling. This change promotes flexibility and modularity.crates/core/src/text_unparser.rs (1)
- 28-28: Changing the
current_nameparameter type toOption<&str>in theapply_effectsfunction is a good practice, as it reduces unnecessary string cloning and memory allocations, potentially improving performance.crates/core/src/pattern/equal.rs (1)
- 79-79: The update to the
execute_funcmethod signature in theEqualstruct, using a trait object for thecontextparameter, aligns with the PR's objectives of abstracting context handling. This enhances flexibility and modularity.crates/core/src/pattern/functions.rs (1)
- 22-22: The changes to use
impl Traitfor thecontextparameter in various trait method signatures, along with adjustments to method calls, align with the PR's objectives of enhancing flexibility and modularity in context handling.Also applies to: 37-37, 52-52, 89-89
crates/core/src/pattern/add.rs (1)
- 69-69: The updates to the
contextparameter type in thecallandevaluatefunctions to useimpl Context<'a>align with the PR's objectives of abstracting context handling. This enhances flexibility and modularity.Also applies to: 79-79
crates/core/src/pattern/divide.rs (1)
- 69-69: The updates to the
contextparameter type in thecallandevaluatefunctions to useimpl Context<'a>align with the PR's objectives of abstracting context handling. This enhances flexibility and modularity.Also applies to: 79-79
crates/core/src/pattern/subtract.rs (1)
- 68-68: The updates to the
contextparameter type in thecallandevaluatefunctions to useimpl Context<'a>align with the PR's objectives of abstracting context handling. This enhances flexibility and modularity.Also applies to: 78-78
crates/core/src/pattern/multiply.rs (2)
- 8-8: The import of
Contextas a trait is aligned with the PR's objective to introduce aContexttrait for more flexible context handling. This change supports the architectural shift towards using trait objects for context, enhancing modularity and flexibility.- 68-68: The update to method signatures to accept a reference to an
impl Context<'a>instead of a concreteContextstruct is consistent with the PR's goal of decoupling and abstracting context handling. This change allows for more versatile implementations of theContexttrait, facilitating easier modifications and extensions in the future.Also applies to: 78-78, 99-99
crates/core/src/pattern/modulo.rs (2)
- 8-8: The inclusion of the
Contexttrait in the imports supports the PR's architectural changes towards a more flexible and abstracted context handling mechanism. This change is crucial for the broader goal of decoupling and modularizing the system.- 68-68: Updating the method signatures to use
&'a impl Context<'a>instead of a concreteContextstruct aligns with the PR's objectives of enhancing modularity and flexibility in context management. This approach allows for theContexttrait to be implemented by various structs, supporting the system's evolving needs.Also applies to: 78-78, 101-101
crates/core/src/pattern/like.rs (2)
- 9-9: Adding the
Contexttrait to the imports is in line with the PR's direction of abstracting context handling. This change facilitates the use of theContexttrait across different parts of the system, promoting a more flexible and modular architecture.- 81-81: The modification of the
Matchertrait implementation to use&'a impl Context<'a>enhances the system's flexibility in handling context. This change is consistent with the PR's objectives of abstracting and decoupling context management, allowing for a wider range of context implementations.Also applies to: 99-99
crates/core/src/pattern/map.rs (2)
- 7-7: The import of the
Contexttrait aligns with the PR's goal of abstracting context handling through traits. This change supports the architectural shift towards more flexible and modular context management.- 78-78: Updating the
Matchertrait implementation forGritMapto use&'a impl Context<'a>is consistent with the PR's objectives. This change allows for more versatile context implementations, supporting the system's modularity and flexibility.crates/core/src/pattern/limit.rs (2)
- 8-8: Importing the
Contexttrait is in line with the PR's direction towards a more abstracted and flexible context handling mechanism. This change is essential for the broader architectural adjustments being made across the system.- 78-78: The update to the
Matchertrait implementation forLimitto use&'a impl Context<'a>and the introduction ofcontext.ignore_limit_pattern()are significant. These changes not only align with the PR's objectives of abstracting context handling but also introduce a functional adjustment to how limit patterns are handled. This approach enhances the system's flexibility and modularity by abstracting the logic for checking limit pattern conditions.Also applies to: 81-81
crates/core/src/pattern/sequential.rs (2)
- 6-6: The import of the
Contexttrait and theStepstruct fromcrate::contextandcrate::patternrespectively, aligns with the PR's objectives of enhancing modularity and flexibility in context management. This change supports the architectural shift towards using trait objects for context.- 89-89: Updating the
Matchertrait implementation forSequentialto use&'a impl Context<'a>is consistent with the PR's goal of abstracting and decoupling context handling. This change allows for more versatile implementations of theContexttrait, facilitating easier modifications and extensions in the future.crates/core/src/pattern/predicate_definition.rs (2)
- 10-10: Importing the
Contexttrait is in line with the PR's direction of abstracting context handling through traits. This change supports the architectural shift towards more flexible and modular context management.- 101-101: The update to the
callmethod signature to use&'a impl Context<'a>instead of a concreteContextstruct aligns with the PR's objectives of enhancing modularity and flexibility in context management. This approach allows for theContexttrait to be implemented by various structs, supporting the system's evolving needs.crates/core/src/pattern/assignment.rs (2)
- 10-10: The import of the
Contexttrait aligns with the PR's goal of abstracting context handling through traits. This change supports the architectural shift towards more flexible and modular context management.- 82-82: Updating the
MatcherandEvaluatortrait implementations forAssignmentto use&'a impl Context<'a>is consistent with the PR's objectives. This change allows for more versatile context implementations, supporting the system's modularity and flexibility.Also applies to: 95-95
crates/core/src/pattern/boolean_constant.rs (2)
- 7-7: The import of the
Contexttrait aligns with the PR's objective to introduce a more abstract and flexible context handling mechanism. Good adjustment.- 44-44: The update to use a generic context type (
impl Context) in theMatchertrait implementation forBooleanConstantis a positive change towards more flexible and modular code. Well done.crates/core/src/pattern/within.rs (1)
- 63-63: Updating the
Matchertrait implementation to use a trait object for thecontextparameter enhances flexibility and aligns with the PR's goal of abstracting context handling. Good job.crates/core/src/pattern/maybe.rs (2)
- 56-56: The use of a trait object for the
contextparameter in theMatchertrait implementation forMaybeis a solid move towards more abstract and flexible context handling.- 111-111: Similarly, using a trait object for the
contextparameter in theEvaluatortrait implementation forPrMaybealigns with the PR's goals and enhances code flexibility.crates/core/src/pattern/pattern_definition.rs (1)
- 102-102: Adopting a trait object for the
contextparameter in thePatternDefinition::callmethod is a commendable change that aligns with the PR's objectives of abstracting context handling.crates/core/src/pattern/string_constant.rs (2)
- 48-48: Using a trait object for the
contextparameter in theMatchertrait implementation forStringConstantis a positive step towards more flexible and abstract context handling.- 92-92: Similarly, the update in the
Matchertrait implementation forAstLeafNodeto use a trait object for thecontextparameter aligns with the PR's objectives and enhances code modularity.crates/core/src/pattern/bubble.rs (1)
- 120-120: The update to use a trait object for the
contextparameter in theMatchertrait implementation forBubbleis a commendable change that aligns with the PR's objectives of abstracting context handling.crates/core/src/pattern/range.rs (1)
- 92-92: Adopting a trait object for the
contextparameter in theMatchertrait implementation forRangeis a positive step towards more flexible and abstract context handling.crates/core/src/suppress.rs (2)
- 14-14: Changing the parameter type for
current_namefromOption<String>toOption<&str>inis_binding_suppressedis a subtle optimization that avoids unnecessary allocations. Good improvement.- 50-50: Similarly, the adjustment in
is_suppress_commentto useOption<&str>forcurrent_nameenhances performance by reducing allocations. Well done.crates/core/src/pattern/and.rs (1)
- 8-10: The import of
Contexthas been updated fromsupertocrate::context, and the function signatures forMatcherandEvaluatorimplementations now use a trait boundimpl Context<'a>instead of a concrete type. This change aligns with the PR's objective to introduce aContexttrait for more flexible context management. However, ensure that all instances whereContextis used have been updated to accommodate this change.crates/core/src/pattern/every.rs (1)
- 6-9: The import statements have been reordered, and the function signatures now correctly use the
Contexttrait. This modification supports the PR's goal of decoupling and abstracting context handling. It's important to verify that these changes are consistently applied across all relevant parts of the codebase to ensure compatibility and maintainability.crates/core/src/pattern/after.rs (1)
- 7-9: The changes in import statements and the update to function signatures to use the
Contexttrait are consistent with the PR's objectives. These adjustments facilitate a more flexible and abstract approach to context management. It's crucial to ensure that the rest of the codebase is aligned with these changes to avoid any integration issues.crates/core/src/pattern/before.rs (1)
- 7-9: The adjustments to import statements and the update to function signatures to use the
Contexttrait inbefore.rsalign with the overarching goal of the PR to abstract context handling. These changes contribute to a more modular and maintainable codebase. Ensure that these modifications are consistently applied and tested across the entire project.crates/core/src/pattern/not.rs (1)
- 9-11: The update to use a trait bound
impl Context<'a>in function signatures and the reorganization of imports innot.rsare in line with the PR's objectives to enhance flexibility and modularity in context management. These changes are crucial for resolving the recursive dependency issue mentioned in the PR description. It's important to ensure that these changes do not introduce any unintended side effects in the codebase.crates/core/src/pattern/any.rs (1)
- 8-10: The modifications in
any.rs, including the reorganization of imports and the update to function signatures to use theContexttrait, support the PR's goal of abstracting context handling. These changes contribute to the system's modularity and maintainability. It's essential to verify that these updates are thoroughly tested and consistent across the codebase.crates/core/src/pattern/some.rs (1)
- 4-11: The update to use a trait bound
impl Context<'a>in theMatcherimplementation withinsome.rsaligns with the PR's objectives to introduce a more flexible and abstract context management system. This change, along with the reorganization of imports, contributes to the decoupling efforts and enhances the codebase's modularity. Ensure that these changes are consistently applied and tested throughout the project.crates/core/src/pattern/or.rs (1)
- 8-10: The changes in
or.rs, including the reorganization of imports and the update to function signatures to use theContexttrait, are consistent with the PR's goal of enhancing context management flexibility. These modifications contribute to the system's overall modularity and maintainability. It's crucial to ensure that these changes are well-integrated and tested across the entire codebase.crates/core/src/pattern/log.rs (4)
- 9-9: The import statement has been reordered and now includes
Contextas a trait object. This change aligns with the PR's objective to introduce aContexttrait for better abstraction and flexibility.- 35-35: The function signature for
add_lognow accepts a reference to animpl Contexttrait object instead of a concreteContexttype. This modification is consistent with the PR's goal to utilize trait objects for more versatile context handling.- 129-129: Similarly, the
executemethod in theMatcherimplementation forLoghas been updated to accept a trait object reference forContext. This change supports the PR's architectural shift towards using trait objects for context management.- 140-140: The
execute_funcmethod in theEvaluatorimplementation forLogalso reflects the PR's direction by accepting a trait object reference forContext. This adjustment is part of the broader effort to decouple and abstract context handling.crates/core/src/pattern/dynamic_snippet.rs (3)
- 10-10: The import statement has been modified to include
Contextdirectly, which simplifies the usage of theContexttrait within this file. This change is part of the PR's effort to streamline context handling.- 55-55: The
textmethod's signature now usesimpl Contextfor thecontextparameter, aligning with the PR's objective to leverage trait objects for more flexible context management.- 74-74: The
executemethod in theMatcherimplementation forDynamicPatternhas been updated to accept a trait object reference forContext, consistent with the PR's architectural changes.crates/core/src/pattern/if.rs (3)
- 10-10: The import statement for
Contexthas been adjusted, which is part of the PR's broader initiative to refactor context handling by introducing aContexttrait.- 96-96: The
executemethod in theMatcherimplementation forIfnow takes a trait reference forContextinstead of a struct reference, aligning with the PR's goal to abstract context handling through trait objects.- 185-185: Similarly, the
execute_funcmethod in theEvaluatorimplementation forPrIfhas been updated to accept a trait object reference forContext, consistent with the PR's architectural changes towards more flexible context management.crates/core/src/pattern/match.rs (2)
- 10-10: The import statement has been reordered and updated to include
Context, which is part of the PR's efforts to refactor context handling by leveraging aContexttrait.- 74-74: The
execute_funcmethod in theEvaluatorimplementation forMatchnow uses a trait object reference forContext, aligning with the PR's objective to abstract context handling through trait objects.crates/core/src/pattern/where.rs (2)
- 13-13: The import statement for
Contexthas been adjusted, reflecting the PR's broader initiative to refactor context handling by introducing aContexttrait for better abstraction.- 158-158: The
executemethod in theMatcherimplementation forWherenow accepts a trait object reference forContext, consistent with the PR's goal to utilize trait objects for more versatile context handling.crates/core/src/pattern/accessor.rs (2)
- 3-3: The import statement now includes
Context, aligning with the PR's objective to refactor context handling by leveraging aContexttrait.- 176-176: The
executemethod in theMatcherimplementation forAccessorhas been updated to accept a trait object reference forContext, consistent with the PR's architectural changes towards more flexible context management.crates/core/src/pattern/list.rs (3)
- 9-9: The import statement has been reorganized to include
Context, which is part of the PR's efforts to refactor context handling by introducing aContexttrait.- 119-119: The
executemethod in theMatcherimplementation forListnow uses a trait object reference forContext, aligning with the PR's objective to abstract context handling through trait objects.- 158-158: The
execute_assocfunction has been updated to accept a trait object reference forContext, consistent with the PR's architectural changes towards more flexible context management.crates/core/src/pattern/ast_node.rs (2)
- 1-1: The import statement now includes
Context, reflecting the PR's broader initiative to refactor context handling by leveraging aContexttrait for better abstraction.- 155-155: The
executemethod in theMatcherimplementation forASTNodehas been updated to use a trait object reference forContext, aligning with the PR's objective to abstract context handling through trait objects.crates/core/src/pattern/regex.rs (4)
- 7-7: The import of
Stateis added. Ensure thatStateis utilized within the file, otherwise, it might be an unnecessary import.- 9-9: The addition of imports from
crate::{binding::Binding, context::Context};is noted. Verify that these imports are necessary and correctly used within the file.- 13-13: The addition of
marzano_language::{language::Language, target_language::TargetLanguage};imports suggests an expansion in language processing capabilities. Ensure these are appropriately integrated into the regex pattern handling.- 150-150: The signature change to accept a trait object
context: &'a impl Context<'a>,in theexecutemethod ofMatcherforRegexPatternis a significant improvement. It enhances flexibility by allowing any type that implements theContexttrait to be passed in, adhering to the Dependency Inversion Principle.crates/core/src/pattern/predicates.rs (2)
- 22-22: The addition of
use crate::context::Context;is appropriate given the changes in the file to accommodate the newContexttrait. This ensures that the file has access to the necessary context-related functionality.- 231-231: Changing the
contextparameter type in theexecute_funcmethod from&Context<'a>to&'a impl Context<'a>is a good practice. It allows for more flexible context implementations to be used, aligning with the Dependency Inversion Principle.crates/core/src/pattern/function_definition.rs (5)
- 1-1: The addition of
use crate::{binding::Constant, context::Context};is necessary for the changes in this file, particularly the use of theContexttrait. This ensures that the necessary types are available for use in the function definitions.- 25-25: The change in the
callmethod signature to usecontext: &'a impl Context<'a>,instead of a concreteContexttype is a positive change. It increases the flexibility and reusability of theFunctionDefinitiontrait by allowing any type that implementsContextto be used.- 115-115: The same change in the
callmethod signature forGritFunctionDefinitionas noted earlier. This consistency across different implementations of theFunctionDefinitiontrait is good for maintainability and flexibility.- 205-205: The conditional compilation directive
#[cfg(not(feature = "external_functions_common"))]is used to provide a fallback implementation of thecallmethod when external functions are not enabled. This is a good practice for feature toggling and ensuring the codebase can adapt to different build configurations.- 215-215: The implementation of the
callmethod under the#[cfg(feature = "external_functions_common")]directive demonstrates a thoughtful approach to handling external function calls, including error handling and type conversions. This detailed implementation ensures robustness and flexibility in function execution.crates/core/src/pattern/accumulate.rs (4)
- 15-15: The addition of
Stateimport is appropriate given the usage ofStatein the file. This ensures that the necessary functionality related to state management is accessible.- 18-18: The addition of
use crate::context::Context;is necessary for the changes in this file to accommodate the newContexttrait. This ensures that the file has access to the necessary context-related functionality.- 147-147: Changing the
contextparameter type in theexecutemethod from&Context<'a>to&'a impl Context<'a>is a good practice. It allows for more flexible context implementations to be used, aligning with the Dependency Inversion Principle.- 225-225: The same change in the
execute_funcmethod signature to usecontext: &'a impl Context<'a>,instead of a concreteContexttype is a positive change. It increases the flexibility and reusability of theEvaluatortrait by allowing any type that implementsContextto be used.crates/core/src/pattern/state.rs (1)
- 240-240: The change in the
bindings_history_to_rangesfunction to acceptcurrent_nameas anOption<&str>instead of&Option<String>is a subtle but meaningful improvement. It simplifies the handling of optional strings and potentially reduces unnecessary allocations.crates/core/src/pattern/list_index.rs (2)
- 11-13: The reordering and addition of imports, including
use crate::context::Context;, are appropriate for the changes in this file. This ensures that the necessary types and functions are available for use in the list index handling.- 237-237: Changing the
contextparameter type in theexecutemethod from&Context<'a>to&'a impl Context<'a>is a good practice. It allows for more flexible context implementations to be used, aligning with the Dependency Inversion Principle.crates/core/src/pattern/rewrite.rs (4)
- 12-12: The import statement for
Contexthas been updated to reflect the introduction of theContexttrait. This change aligns with the PR objectives of enhancing modularity and flexibility in context management.- 183-183: The function signature of
execute_generalizedhas been modified to accept a reference to an implementation of theContexttrait instead of a struct. This change is crucial for achieving the desired abstraction and flexibility in handling context. However, it's important to ensure that all implementations of theContexttrait provide the necessary functionality expected by this method.- 260-260: The function signature of
executein theMatcherimplementation forRewritehas been updated to use theContexttrait. This change is consistent with the PR's goal of decoupling and abstracting context handling. Ensure that theexecutemethod's logic is compatible with all potential implementations of theContexttrait.- 271-271: The function signature of
execute_funcin theEvaluatorimplementation forRewritehas been updated to use theContexttrait. This change supports the PR's objectives by abstracting context handling. It's essential to verify that theexecute_funcmethod functions correctly with all implementations of theContexttrait.crates/core/src/pattern/contains.rs (3)
- 1-1: The import statement for
Contexthas been updated to reflect the introduction of theContexttrait. This change is in line with the PR's objectives of enhancing modularity and flexibility in context management.- 78-78: The function signature of
execute_untilhas been modified to accept a reference to an implementation of theContexttrait instead of a struct. This change supports the PR's goal of abstracting context handling. Ensure that theexecute_untilmethod's logic is compatible with all potential implementations of theContexttrait.- 134-134: The function signature of
executein theMatcherimplementation forContainshas been updated to use theContexttrait. This change aligns with the PR's objectives by decoupling and abstracting context handling. It's important to verify that theexecutemethod functions correctly with all implementations of theContexttrait.crates/core/src/pattern/step.rs (10)
- 15-15: The import of the
Contexttrait aligns with the PR's objective to introduce a newContexttrait for more flexible context handling. This change is foundational and supports the subsequent modifications in the file.- 193-193: Changing the type of the
contextparameter in theexecutemethod to&'a impl Context<'a>is a significant architectural change. It replaces the concrete type with a trait, allowing for more flexible implementations of the context. This change is crucial for achieving the PR's goal of enhancing modularity and flexibility in context management.- 197-197: Updating the method call to
context.language().get_ts_language()with parentheses correctly adapts to the newContexttrait's method signature. This change ensures that the code remains functional after the introduction of the trait.- 224-224: The method call
state.bindings_history_to_ranges(context.language(), context.name())has been updated to use the newContexttrait methods. This change is consistent with the PR's objectives and demonstrates the application of the trait's methods in practice.- 261-262: The method calls
context.language()andcontext.name()have been correctly updated to match the newContexttrait's method signatures. This ensures that the code remains functional and aligns with the PR's architectural changes.- 267-267: The method call
get_orphaned_ranges(&tree, &new_src, context.language())correctly uses theContexttrait'slanguagemethod. This change is consistent with the PR's objectives and demonstrates the flexibility provided by theContexttrait.- 283-283: The method call
context.language()within theFileOwner::newconstructor correctly adapts to the newContexttrait's method signature. This ensures that the code remains functional after the introduction of the trait.- 286-286: The method call
context.files().push(owned_file)demonstrates the use of theContexttrait'sfilesmethod. This change aligns with the PR's goal of enhancing modularity and flexibility in context management.- 289-289: The method call
context.files().last().unwrap()within thestate.files.push_revisioncall correctly uses theContexttrait'sfilesmethod. This ensures that the code remains functional and aligns with the PR's architectural changes.- 302-304: The method calls
context.language(),context.files().push(owned_file), andstate.files.push_new_file(context.files().last().unwrap())correctly adapt to the newContexttrait's method signatures. These changes are consistent with the PR's objectives and demonstrate the flexibility provided by theContexttrait.crates/core/src/pattern/built_in_functions.rs (7)
- 3-3: The import of
context::Contextaligns with the PR's objective to introduce aContexttrait, facilitating more flexible context management.- 62-62: Changing the function signature to accept
&'a impl Context<'a>instead of a concreteContexttype is a positive change. It increases flexibility and decouples the function from a specific context implementation, aligning with the PR's objectives.- 79-79: The modification of the
Ftype to accept a simplified argument list is a good practice. It simplifies the function signatures and makes the code more readable and maintainable.- 92-92: Updating function signatures to accept
Vec<Option<ResolvedPattern<'a>>>instead of&'a [Option<Pattern>]is a significant change. This adjustment is necessary due to the introduction of theContexttrait and the refactoring of built-in functions. It simplifies the handling of patterns and enhances the flexibility of function arguments.- 119-119: The use of
&'a impl Context<'a>in thecallmethod ofBuiltInsstruct is consistent with the changes made throughout the file. It demonstrates the application of theContexttrait, promoting a more abstract and flexible approach to context management.- 181-181: The changes in argument types for built-in functions, such as
resolve_path_fn, to acceptVec<Option<ResolvedPattern<'a>>>instead of the previous types, are in line with the PR's objectives. These changes are necessary for the new context handling mechanism and improve the code's consistency.- 204-204: The simplification of function implementations, as seen in
capitalize_fn, by directly working with the provided arguments and removing redundant handling, enhances code readability and maintainability. This change is part of the broader refactoring effort and aligns with best practices.crates/core/src/pattern.rs (3)
- 28-28: Promoting modules such as
function_definitionto be public (pub) is a significant change. This adjustment likely supports the broader architectural changes by making these modules accessible from other parts of the codebase. Ensure that this change aligns with the intended design and does not inadvertently expose internal details that should remain encapsulated.Verification successful
The search results indicate extensive references to
function_definitionacross various parts of the codebase, including withincrates/core/src/where it is used in multiple files such ascontext.rs,test.rs,functions.rs,function_definition.rs,call.rs, andcompiler.rs. These references are not only in the context of module imports but also in the usage of types, functions, and structures related tofunction_definition. This demonstrates that the modulefunction_definitionbeing made public is indeed utilized throughout the codebase, aligning with the intended design and justifying its exposure for broader architectural integration.Given the evidence from the search results, it can be concluded that the change to make the
function_definitionmodule public is justified and aligns with its usage across the codebase. This supports the initial review comment regarding the significance of such changes and their alignment with broader architectural goals.* 775-775: Renaming `Context` to `MarzanoContext` is a foundational change that aligns with the PR's objectives. This renaming sets the stage for further modifications and clarifies the role of this struct within the new architectural design. Ensure that all references to the old `Context` struct have been updated accordingly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the newly public modules are used appropriately in other parts of the codebase. fd 'pattern.rs' --exec grep -Hn 'pub mod function_definition;'Length of output: 123
Script:
#!/bin/bash # Search for references to `function_definition` module across the codebase, excluding its declaration in `pattern.rs`. rg --glob '!crates/core/src/pattern.rs' 'function_definition'Length of output: 46968
Verification successful
The references to
Context<'a>found in the script output are expected and align with the new architectural design, whereContextis used as a trait. The renaming ofContexttoMarzanoContextand the updates to references have been correctly handled, indicating that the renaming and subsequent modifications are consistent with the PR's objectives.* 814-852: The implementation of the `Context` trait for `MarzanoContext` is a crucial part of the PR's objectives. It encapsulates a wide range of functionalities, including access to patterns, predicates, function definitions, and more. This implementation abstracts and generalizes context interactions across the system, promoting flexibility and modularity. Ensure that all required methods are implemented correctly and that the trait's contract is fully satisfied.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the old `Context` struct to ensure they've been updated. rg --type rust 'Context<' || echo "No references to the old Context struct found."Length of output: 8459
crates/core/src/pattern/resolved_pattern.rs (3)
- 1-24: The reorganization of imports improves code readability and aligns with the architectural changes introduced in this PR. Good job on keeping the imports clean and organized.
- 549-549: Adjusting function signatures to use the
Contexttrait is consistent with the PR's objectives of abstracting context handling. Ensure that the dynamic nature of trait objects does not introduce performance overhead or type safety issues in critical paths.- 580-586: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [549-657]
The minor code restructuring changes, including adjustments to how patterns and snippets are resolved, align well with the PR's objectives and improve code organization and flexibility. These changes appear to be carefully considered and should not introduce any issues.
crates/core/src/pattern/patterns.rs (4)
- 61-61: The change from
&Context<'a>toimpl Context<'a>in theMatchertrait'sexecutemethod signature is a significant architectural adjustment. This change introduces more flexibility in how context can be implemented and passed around, aligning with the PR's objectives to enhance modularity and flexibility in context management. However, ensure that all implementors of this trait and callers of theexecutemethod are updated accordingly to accommodate this change.- 587-587: Similar to the
Matchertrait, thePatternimplementation'stextmethod now usesimpl Context<'a>instead of&Context<'a>. This change is consistent with the PR's goal of abstracting context handling and should offer improved flexibility. It's crucial to verify that all usages of this method are updated to reflect this new signature.- 598-598: The
floatmethod in thePatternimplementation also adopts theimpl Context<'a>signature. This consistency across method signatures is beneficial for the architectural changes aimed at in the PR. As with other changes, ensure that all relevant method calls are reviewed and updated as necessary.- 1053-1053: The
executemethod in thePatternimplementation now usesimpl Context<'a>, aligning with the changes observed in theMatchertrait and other methods withinPattern. This change supports the PR's objectives towards a more flexible and modular architecture. Confirm that the implementation and usage of this method across the codebase are compatible with this change.
crates/core/src/context.rs
Outdated
Comment on lines
10
to
30
| pub trait Context<'a> { | ||
| fn pattern_definitions(&self) -> &[PatternDefinition]; | ||
|
|
||
| fn predicate_definitions(&self) -> &[PredicateDefinition]; | ||
|
|
||
| fn function_definitions(&self) -> &[GritFunctionDefinition]; | ||
|
|
||
| fn foreign_function_definitions(&self) -> &[ForeignFunctionDefinition]; | ||
|
|
||
| fn ignore_limit_pattern(&self) -> bool; | ||
|
|
||
| // FIXME: Don't depend on Grit's file handling in Context. | ||
| fn files(&self) -> &FileOwners; | ||
|
|
||
| fn built_ins(&self) -> &BuiltIns; | ||
|
|
||
| // FIXME: This introduces a dependency on TreeSitter. | ||
| fn language(&self) -> &TargetLanguage; | ||
|
|
||
| fn name(&self) -> Option<&str>; | ||
| } |
There was a problem hiding this comment.
The Context trait is well-defined, covering a comprehensive set of methods for accessing various components. This abstraction is crucial for the PR's goal of enhancing modularity and flexibility. However, the FIXME comments on lines 21 and 26 highlight dependencies that may need to be addressed. Consider creating separate issues to track these dependencies and explore ways to decouple them from the Context trait.
Would you like me to open GitHub issues to track the resolution of the FIXME comments regarding dependencies on Grit's file handling and TreeSitter?
Unfortunately this is somewhat of an issue. The context struct is only ever used (or at the very least introduced) for built in functions. The functions they are used in are in a closed source ai_builtins crate. |
morgante
reviewed
Mar 25, 2024
I’ll try if I can come up with a different solution to this then! |
|
I've updated the PR so that |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (59)
- crates/core/src/context.rs (1 hunks)
- crates/core/src/pattern.rs (8 hunks)
- crates/core/src/pattern/accessor.rs (3 hunks)
- crates/core/src/pattern/accumulate.rs (6 hunks)
- crates/core/src/pattern/add.rs (4 hunks)
- crates/core/src/pattern/after.rs (3 hunks)
- crates/core/src/pattern/and.rs (3 hunks)
- crates/core/src/pattern/any.rs (3 hunks)
- crates/core/src/pattern/assignment.rs (3 hunks)
- crates/core/src/pattern/ast_node.rs (4 hunks)
- crates/core/src/pattern/before.rs (3 hunks)
- crates/core/src/pattern/boolean_constant.rs (2 hunks)
- crates/core/src/pattern/bubble.rs (2 hunks)
- crates/core/src/pattern/built_in_functions.rs (17 hunks)
- crates/core/src/pattern/call.rs (4 hunks)
- crates/core/src/pattern/code_snippet.rs (2 hunks)
- crates/core/src/pattern/contains.rs (3 hunks)
- crates/core/src/pattern/divide.rs (4 hunks)
- crates/core/src/pattern/dynamic_snippet.rs (4 hunks)
- crates/core/src/pattern/equal.rs (2 hunks)
- crates/core/src/pattern/every.rs (2 hunks)
- crates/core/src/pattern/file_pattern.rs (2 hunks)
- crates/core/src/pattern/files.rs (2 hunks)
- crates/core/src/pattern/float_constant.rs (2 hunks)
- crates/core/src/pattern/function_definition.rs (5 hunks)
- crates/core/src/pattern/functions.rs (5 hunks)
- crates/core/src/pattern/if.rs (3 hunks)
- crates/core/src/pattern/includes.rs (2 hunks)
- crates/core/src/pattern/int_constant.rs (2 hunks)
- crates/core/src/pattern/like.rs (3 hunks)
- crates/core/src/pattern/limit.rs (2 hunks)
- crates/core/src/pattern/list.rs (3 hunks)
- crates/core/src/pattern/list_index.rs (3 hunks)
- crates/core/src/pattern/log.rs (4 hunks)
- crates/core/src/pattern/map.rs (2 hunks)
- crates/core/src/pattern/match.rs (2 hunks)
- crates/core/src/pattern/maybe.rs (4 hunks)
- crates/core/src/pattern/modulo.rs (4 hunks)
- crates/core/src/pattern/multiply.rs (4 hunks)
- crates/core/src/pattern/not.rs (3 hunks)
- crates/core/src/pattern/or.rs (3 hunks)
- crates/core/src/pattern/pattern_definition.rs (2 hunks)
- crates/core/src/pattern/patterns.rs (5 hunks)
- crates/core/src/pattern/predicate_definition.rs (2 hunks)
- crates/core/src/pattern/predicate_return.rs (2 hunks)
- crates/core/src/pattern/predicates.rs (2 hunks)
- crates/core/src/pattern/range.rs (2 hunks)
- crates/core/src/pattern/regex.rs (2 hunks)
- crates/core/src/pattern/resolved_pattern.rs (8 hunks)
- crates/core/src/pattern/rewrite.rs (4 hunks)
- crates/core/src/pattern/sequential.rs (3 hunks)
- crates/core/src/pattern/some.rs (2 hunks)
- crates/core/src/pattern/step.rs (6 hunks)
- crates/core/src/pattern/string_constant.rs (3 hunks)
- crates/core/src/pattern/subtract.rs (4 hunks)
- crates/core/src/pattern/undefined.rs (2 hunks)
- crates/core/src/pattern/variable.rs (2 hunks)
- crates/core/src/pattern/where.rs (2 hunks)
- crates/core/src/pattern/within.rs (2 hunks)
Files skipped from review as they are similar to previous changes (57)
- crates/core/src/context.rs
- crates/core/src/pattern.rs
- crates/core/src/pattern/accessor.rs
- crates/core/src/pattern/accumulate.rs
- crates/core/src/pattern/add.rs
- crates/core/src/pattern/after.rs
- crates/core/src/pattern/and.rs
- crates/core/src/pattern/any.rs
- crates/core/src/pattern/assignment.rs
- crates/core/src/pattern/ast_node.rs
- crates/core/src/pattern/before.rs
- crates/core/src/pattern/boolean_constant.rs
- crates/core/src/pattern/bubble.rs
- crates/core/src/pattern/built_in_functions.rs
- crates/core/src/pattern/call.rs
- crates/core/src/pattern/code_snippet.rs
- crates/core/src/pattern/contains.rs
- crates/core/src/pattern/divide.rs
- crates/core/src/pattern/dynamic_snippet.rs
- crates/core/src/pattern/equal.rs
- crates/core/src/pattern/every.rs
- crates/core/src/pattern/file_pattern.rs
- crates/core/src/pattern/files.rs
- crates/core/src/pattern/float_constant.rs
- crates/core/src/pattern/function_definition.rs
- crates/core/src/pattern/functions.rs
- crates/core/src/pattern/if.rs
- crates/core/src/pattern/includes.rs
- crates/core/src/pattern/int_constant.rs
- crates/core/src/pattern/like.rs
- crates/core/src/pattern/limit.rs
- crates/core/src/pattern/list.rs
- crates/core/src/pattern/list_index.rs
- crates/core/src/pattern/map.rs
- crates/core/src/pattern/match.rs
- crates/core/src/pattern/maybe.rs
- crates/core/src/pattern/modulo.rs
- crates/core/src/pattern/not.rs
- crates/core/src/pattern/or.rs
- crates/core/src/pattern/pattern_definition.rs
- crates/core/src/pattern/patterns.rs
- crates/core/src/pattern/predicate_definition.rs
- crates/core/src/pattern/predicate_return.rs
- crates/core/src/pattern/predicates.rs
- crates/core/src/pattern/range.rs
- crates/core/src/pattern/regex.rs
- crates/core/src/pattern/resolved_pattern.rs
- crates/core/src/pattern/rewrite.rs
- crates/core/src/pattern/sequential.rs
- crates/core/src/pattern/some.rs
- crates/core/src/pattern/step.rs
- crates/core/src/pattern/string_constant.rs
- crates/core/src/pattern/subtract.rs
- crates/core/src/pattern/undefined.rs
- crates/core/src/pattern/variable.rs
- crates/core/src/pattern/where.rs
- crates/core/src/pattern/within.rs
Additional comments: 4
crates/core/src/pattern/multiply.rs (2)
- 8-8: The import of
Contextaligns with the PR's objective to introduce aContexttrait for flexible context management. This change is crucial for decoupling and enhancing modularity within the system.- 68-68: The function signatures for
call,evaluate, andexecutenow accept a reference to an implementation of theContexttrait, rather than a concreteContextstruct. This is a significant architectural improvement, promoting flexibility and decoupling of components. However, ensure that all implementations of theContexttrait provide the necessary functionality expected by these methods.Also applies to: 78-78, 99-99
crates/core/src/pattern/log.rs (2)
- 9-9: The import adjustments and the use of the
Contexttrait in thelog.rsfile are in line with the PR's objectives to refine dependencies and parameter types for better abstraction and flexibility in handling context-related functionality. This change supports the broader goal of making the system more adaptable.- 35-35: The function signatures for
add_log,execute, andexecute_funcnow accept a reference to an implementation of theContexttrait. This change enhances the flexibility and decoupling of theLogfunctionality from specific context implementations. It's important to verify that all necessary context interactions are supported by theContexttrait to ensure these methods function as intended.Also applies to: 129-129, 140-140
morgante
approved these changes
Mar 26, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- crates/core/src/pattern/includes.rs (2 hunks)
- crates/core/src/pattern/patterns.rs (5 hunks)
- crates/core/src/pattern/regex.rs (3 hunks)
Files skipped from review as they are similar to previous changes (3)
- crates/core/src/pattern/includes.rs
- crates/core/src/pattern/patterns.rs
- crates/core/src/pattern/regex.rs
|
@arendjr This is going to need more work:
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (7)
- crates/core/src/context.rs (1 hunks)
- crates/core/src/parse.rs (3 hunks)
- crates/core/src/pattern.rs (8 hunks)
- crates/core/src/pattern/function_definition.rs (6 hunks)
- crates/core/src/suppress.rs (3 hunks)
- crates/language/src/target_language.rs (1 hunks)
- crates/util/src/runtime.rs (5 hunks)
Files skipped from review as they are similar to previous changes (4)
- crates/core/src/context.rs
- crates/core/src/pattern.rs
- crates/core/src/pattern/function_definition.rs
- crates/core/src/suppress.rs
Additional comments: 10
crates/core/src/parse.rs (5)
- 1-4: Reorganizing imports and using
anyhow::Resultdirectly improves readability and reduces the need for explicit error type declarations. This change aligns with Rust's idiomatic practices for error handling.- 9-11: Adding specific imports within the function scope is a good practice when they are only used in a particular context. This keeps the global namespace cleaner and reduces the cognitive load when understanding the file's dependencies.
- 28-30: The error message provided in the
parse_input_filefunction for thenot(feature = "grit-parser")case is clear and informative. This is a good practice for feature-flagged functionality, guiding the user on how to resolve the issue.- 38-39: Using
anyhow::Contextfor error handling inmake_grit_parserunder thegrit-parserfeature flag is a good practice. It provides more context on the error, making debugging easier.- 49-50: Similar to the previous comment, the error handling for the
not(feature = "grit-parser")case inmake_grit_parseris clear and guides the user on enabling the necessary feature flag.crates/util/src/runtime.rs (4)
- 26-31: Defining types for functions
FetchFnandExecExternalFnunder specific feature flags and target architecture is a good practice. It enhances code readability and ensures type safety for function pointers used in different configurations.- 104-104: The constructor
newforExecutionContextunder thenetwork_requests_externalandexternal_functions_ffifeature flags is well-defined. It initializes the struct with provided function pointers, which allows for flexible and configurable runtime behavior.- 126-128: The error handling in
send_requestfor thenetwork_requestsfeature flag usesanyhow::anyhow!to provide a clear error message when the Tokio runtime handle is not available. This is a good practice for providing actionable error messages to the developer.- 3-3: Consider using environment variable management libraries like
dotenvfor handling environment variables. This can provide a more structured approach to managing environment configurations and improve code maintainability.crates/language/src/target_language.rs (1)
- 168-168: The change in the
get_language_with_parserfunction signature to use underscore placeholders indicates that the parameters are no longer used. Ensure that this change aligns with the intended functionality and that any dependent code has been updated accordingly.
crates/core/src/parse.rs
Outdated
Comment on lines
1
to
15
| use crate::pattern::api::InputFile; | ||
| use anyhow::Result; | ||
| use marzano_language::{language::Language, target_language::TargetLanguage}; | ||
| use serde_json::to_string_pretty; | ||
| use marzano_language::target_language::TargetLanguage; | ||
| use std::path::Path; | ||
| use tree_sitter::Parser; | ||
|
|
||
| use crate::{pattern::api::InputFile, tree_sitter_serde::tree_sitter_node_to_json}; | ||
|
|
||
| #[cfg(feature = "grit-parser")] | ||
| pub fn parse_input_file(lang: &TargetLanguage, input: &str, path: &Path) -> Result<InputFile> { | ||
| use crate::tree_sitter_serde::tree_sitter_node_to_json; | ||
| use marzano_language::language::Language; | ||
| use serde_json::to_string_pretty; | ||
|
|
||
| let mut parser = Parser::new().unwrap(); | ||
| parser.set_language(lang.get_ts_language()).unwrap(); | ||
| let tree = parser.parse(input.as_bytes(), None).unwrap().unwrap(); |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [12-27]
The use of unwrap() in multiple places within parse_input_file function poses a risk of panicking at runtime. Consider handling these potential errors gracefully to improve the robustness of the code.
- let mut parser = Parser::new().unwrap();
- parser.set_language(lang.get_ts_language()).unwrap();
- let tree = parser.parse(input.as_bytes(), None).unwrap().unwrap();
- let input_file_debug_text = to_string_pretty(&tree_sitter_node_to_json(
- &tree.root_node(),
- input,
- Some(lang),
- ))
- .unwrap();
+ let mut parser = Parser::new().context("Failed to create new parser")?;
+ parser.set_language(lang.get_ts_language()).context("Failed to set language for parser")?;
+ let tree = parser.parse(input.as_bytes(), None).context("Failed to parse input")?.context("Parsed tree is empty")?;
+ let input_file_debug_text = to_string_pretty(&tree_sitter_node_to_json(&tree.root_node(), input, Some(lang)))
+ .context("Failed to convert tree to pretty JSON string")?;
|
WASM builds should be fixed now. The Clippies weren't related to my PR, but it looks like they simply only showed up with WASM builds. Fixed too. |
|
@arendjr FYI you don't have to listen to the AI rabbit unless you want to. We're just trying it out - so far it seems to mostly add noise. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR renames the
Contextstruct toMarzanoContextand introduces a newContexttrait to take its place. While this PR doesn't accomplish much on its own, the end goal is to be able to extract the patterns and matching engine frommarzano-coreand put them into a crate of their own (grit-core-patterns?). This crate could in turn be used by Biome for its plugin engine.There is one change in this PR that might be slightly controversial: I've refactored
builtin_functions.rsa bit (see the first commit) so that context is no longer passed to built-in functions. This creates no issues now, but could possibly create limitations with future functions. But the reason I did this is that otherwise theBuiltinstype would need to have a generic argument for the context, and because the context can return the built-ins, this created a recursive dependency. This got me into a fight with the borrow-checker I was not able to resolve otherwise.If this PR is approved, I will continue decoupling stuff until I'm able to extract the
grit-core-patternscrate without having dependencies on Tree-Sitter or other Marzano crates. Please let me know if there are concerns with this strategy.Summary by CodeRabbit
Summary by CodeRabbit
Contexttrait with comprehensive methods for enhanced pattern, predicate, and function handling.MarzanoContextto align with theContexttrait.suppress.rsandtext_unparser.rsfor more efficient string processing.