-
Notifications
You must be signed in to change notification settings - Fork 87
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
chore: consolidate as_node()
#89
Conversation
WalkthroughThe recent updates focus on refining the handling of bindings and nodes within a codebase, particularly emphasizing the correct identification and processing of file names and node types. Changes streamline the interface for retrieving node information from bindings, ensuring more direct and type-safe operations. Additionally, a subtle yet impactful adjustment in the handling of text padding enhances the precision of pattern resolution, contributing to overall codebase efficiency and clarity. 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 (
|
@@ -989,7 +989,7 @@ impl<'a> ResolvedPattern<'a> { | |||
let Some(ResolvedSnippet::Text(text)) = snippets.front() else { | |||
return Ok(()); | |||
}; | |||
if let Some(padding) = binding.get_insertion_padding(&text, is_first, language) { | |||
if let Some(padding) = binding.get_insertion_padding(text, is_first, language) { |
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.
Resolves a Clippy.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- crates/core/src/binding.rs (2 hunks)
- crates/core/src/pattern/after.rs (1 hunks)
- crates/core/src/pattern/before.rs (1 hunks)
- crates/core/src/pattern/resolved_pattern.rs (1 hunks)
Additional comments: 5
crates/core/src/pattern/after.rs (1)
- 58-58: The replacement of
get_node()
withas_node()
aligns with the PR's objective to standardize node retrieval. Ensure thatas_node()
comprehensively handles all scenarios previously covered byget_node()
, including error handling and edge cases.crates/core/src/pattern/before.rs (1)
- 58-58: The replacement of
get_node()
withas_node()
aligns with the PR's objective to standardize node retrieval. Ensure thatas_node()
comprehensively handles all scenarios previously covered byget_node()
, including error handling and edge cases.crates/core/src/binding.rs (2)
- 478-485: The modification to the
as_filename
method simplifies its logic by directly returning the path for aFileName
variant, which is a positive change that enhances clarity and efficiency.- 487-492: The redefinition of the
as_node
method to return aNodeWithSource
for node type bindings aligns with the PR's objective to standardize node handling. Ensure thatas_node()
comprehensively handles all scenarios previously covered by its old definition.crates/core/src/pattern/resolved_pattern.rs (1)
- 992-992: The change in how
get_insertion_padding
is called, specifically the handling of thetext
parameter, may affect the logic related to padding insertion. It's important to verify that this change does not introduce regressions or unintended behavior in padding insertion logic.
See discussion at: #85 (comment)
I've followed @ilevyor 's suggestion to return
None
for the empty and list variants, although I ended up calling itas_node()
for consistency with theas_filename()
method we also have now.Summary by CodeRabbit