feat: include content in match results #391
Conversation
WalkthroughWalkthroughThis batch of changes primarily revolves around refactoring the way content is extracted and handled across various modules. Key improvements include transitioning from directly accessing content fields to using method calls, refining the handling of byte ranges, and enhancing the Changes
Sequence Diagram(s)No sequence diagrams are provided as the changes focus on refactoring and slight adjustments rather than introducing new features or significantly altering control flows. Recent review detailsConfiguration used: .coderabbit.yaml Files ignored due to path filters (32)
Files selected for processing (14)
Additional context usedLearnings (1)
Additional comments not posted (31)
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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}
| @@ -219,7 +219,7 @@ impl<'a> Binding<'a, MarzanoQueryContext> for MarzanoBinding<'a> { | |||
| match self { | |||
| Self::Empty(_, _) => None, | |||
| Self::Node(node) => Some(node.range()), | |||
| Self::String(source, range) => Some(Range::from_byte_range(source, *range)), | |||
| Self::String(source, range) => Some(Range::from_byte_range(source, range)), | |||
There was a problem hiding this comment.
Tip
Codebase Verification
Instances of direct field access still present
There are still several instances of direct field access using .byte_range across the codebase. Please update these occurrences to use Range::from_byte_range for consistency and improved code quality.
crates/marzano_messenger/src/format.rs: if let Some(byte_ranges) = &res.rewritten.byte_rangescrates/grit-util/src/language.rs: Some(node.byte_range())crates/core/src/marzano_binding.rs: Self::Node(node) => Some(node.byte_range())crates/core/src/api.rs: file.matches.borrow().byte_ranges.as_ref()crates/core/src/marzano_resolved_pattern.rs: let range = root.byte_range()crates/core/src/pattern_compiler/bubble_compiler.rs: .map(|n| Ok((n.text()?.trim().to_string(), n.byte_range())))crates/core/src/pattern_compiler/variable_compiler.rs: let range = node.byte_range()crates/core/src/pattern_compiler/compiler.rs: .map(|n| Ok((n.text()?.trim().to_string(), n.byte_range())))crates/core/src/pattern_compiler/pattern_compiler.rs: let range = node.byte_range()crates/core/src/pattern_compiler/pattern_compiler.rs: vec![child.byte_range()]crates/core/src/pattern_compiler/snippet_compiler.rs: source[last..byte_range.start].to_string()
Analysis chain
LGTM!
The use of Range::from_byte_range(source, range) improves the code by leveraging a method for creating a Range from a byte range. Ensure that all other instances of direct field access are updated accordingly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all instances of direct field access are updated to use `Range::from_byte_range`.
# Test: Search for direct field access. Expect: No occurrences of direct field access.
rg --type rust $'\.byte_range'
Length of output: 1100
| @@ -439,7 +440,7 @@ mod tests { | |||
| .unwrap(); | |||
|
|
|||
| if let MatchResult::Rewrite(rewrite) = rewrite { | |||
| assert_snapshot!(rewrite.rewritten.content); | |||
| assert_snapshot!(rewrite.content().unwrap()); | |||
There was a problem hiding this comment.
Tip
Codebase Verification
Improvement: Safe content access using content method with potential risk.
The function now uses the content method from the MatchResult struct to access content safely. However, the usage of unwrap() on the returned Option without handling the None case might lead to runtime panics if the content is missing.
- Consider handling the
Nonecase to avoid potential runtime panics.
Analysis chain
Improvement: Safe content access using content method.
The function now uses the content method from the MatchResult struct to access content safely. This change ensures that the content is handled correctly and avoids potential issues with missing content.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all function calls to `content` method in `crates/core/src/test_notebooks.rs`.
# Test: Search for the function usage. Expect: Only occurances of the new method.
rg --type rust $'.content'
Length of output: 55761
Script:
#!/bin/bash
# Description: Verify all function calls to `content` method from `MatchResult` struct in `crates/core/src/test_notebooks.rs`.
# Test: Search for the specific method usage.
rg --type rust 'MatchResult.*\.content\(\)' crates/core/src/test_notebooks.rs
Length of output: 79
Script:
#!/bin/bash
# Description: Verify all function calls to `content` method in `crates/core/src/test_notebooks.rs`.
# Test: Search for the specific method usage.
rg --type rust '\.content\(\)' crates/core/src/test_notebooks.rs
Length of output: 630
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM
30 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Instead of trying to reliably retrieve the original content later, we serialize it into the match results.
Summary by CodeRabbit
Bug Fixes
Refactor
Tests
Greptile Summary
This is an auto-generated summary
MatchResultenum; removeFileMatchstruct.contentmethod fromMatchResultfor file content retrieval.extract_original_contentfunction; updateformat_result_diff.apply_rewriteto usecontentmethod fromFileMatchResult.contentandbyteRangesfields to match results for reliable content retrieval.