-
Notifications
You must be signed in to change notification settings - Fork 6
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
implemented NodeRef::is_match
and NodeRef::is
#53
Conversation
WalkthroughThe pull request introduces new methods Changes
Possibly related PRs
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #53 +/- ##
==========================================
+ Coverage 91.54% 91.56% +0.02%
==========================================
Files 14 14
Lines 2484 2490 +6
==========================================
+ Hits 2274 2280 +6
Misses 210 210 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/node-traversal.rs (1)
227-234
: Great test coverage
The newtest_node_is()
function clearly verifies that theNodeRef::is
method correctly matches the#parent
node against both its ID-based selector (div#parent
) and a pseudo-class selector (:has(#first-child)
). Including a negative test (e.g., asserting a different selector returnsfalse
) would further strengthen coverage.src/node/node_ref.rs (1)
652-653
: Defensive selector handling
By returningfalse
on a failedMatcher::new(sel)
(e.g., invalid selector), the user experience is graceful. Consider logging or warning if invalid selectors might cause confusion in debugging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md
(1 hunks)src/node/node_ref.rs
(2 hunks)tests/node-traversal.rs
(1 hunks)
🔇 Additional comments (3)
tests/node-traversal.rs (1)
225-226
: Looks good!
The test attributes are correctly applied for both #[cfg_attr(not(target_arch = "wasm32"), test)]
and #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
, ensuring cross-platform testing.
src/node/node_ref.rs (1)
647-649
: Method is_match
is well-implemented
This method succinctly delegates to matcher.match_element(self)
, making the code easy to follow and maintain.
CHANGELOG.md (1)
7-11
: Changelog entry is clear
Thank you for documenting the addition of NodeRef::is_match
and NodeRef::is
. This entry provides a concise summary for users to quickly understand and adopt the new methods.
Summary by CodeRabbit
is_match
andis
methods toNodeRef
for checking node matching against matchers and selectors