-
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
Examples.md: added more examples #59
Conversation
WalkthroughThis pull request introduces new methods and documentation for the 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 #59 +/- ##
=======================================
Coverage 91.85% 91.85%
=======================================
Files 15 15
Lines 2577 2577
=======================================
Hits 2367 2367
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: 1
🧹 Nitpick comments (1)
Examples.md (1)
726-758
: Consider adding error handling examples.The example effectively demonstrates the
find()
method usage. However, it would be beneficial to show how the method handles invalid paths (e.g., paths that start before the current node).Consider adding an example that demonstrates error handling:
// This will fail as the path starts before the current node let invalid_path = &["html", "body", "div", "a"]; assert!(main_node.find(invalid_path).is_empty()); // This will work as the path starts after the current node let valid_path = &["div", "a"]; assert_eq!(main_node.find(valid_path).len(), total_links);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Examples.md
(1 hunks)README.md
(1 hunks)src/entities.rs
(1 hunks)tests/node-traversal.rs
(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- tests/node-traversal.rs
- src/entities.rs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
Examples.md (2)
646-682
: LGTM! Clear and comprehensive example.The example effectively demonstrates the
base_uri()
method usage at both document and node level, with helpful comments explaining the implementation details and performance considerations.
685-723
: LGTM! Well-structured example with clear explanations.The example effectively demonstrates the versatility of the
is()
method for both Selection and NodeRef, with clear comments explaining the differences and use cases.README.md (1)
666-779
: LGTM! Documentation is consistent across files.The examples in README.md perfectly mirror those in Examples.md, maintaining consistency in the documentation. This is excellent for user experience as it provides multiple entry points to learn about the new features.
Examples.md
Outdated
</head> | ||
<body> | ||
<div id="main"></div> | ||
</div> |
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.
Fix the typo: Remove extra closing div tag.
There's an extra closing </div>
tag that should be removed.
- </div>
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 (3)
Examples.md (3)
646-681
: Add error handling examples for base_uri().The example demonstrates successful cases but doesn't show how to handle cases where the base URI is missing. Consider adding examples that demonstrate:
- Error handling when no base element exists
- Handling malformed href attributes
// Example: Error handling let doc = Document::from("<html><head></head></html>"); assert!(doc.base_uri().is_none()); // No base element let doc = Document::from(r#"<html><head><base href="invalid://uri"/></head></html>"#); if let Some(uri) = doc.base_uri() { println!("Base URI: {}", uri); } else { println!("No valid base URI found"); }
684-722
: Enhance selector examples for is() method.The example could be enriched with more complex selector patterns to better demonstrate the method's capabilities:
// Example: Complex selectors assert!(main_sel.is("div#main:not(.hidden)[dir]")); assert!(main_node.is("html > body > div#main:first-child:not(:empty)")); // Example: Combining multiple checks assert!(main_sel.is("div#main") && main_sel.is("[dir=ltr]"));
725-757
: Add performance comparison and more path patterns.The example could be enhanced with:
- Performance comparison between find() and select()
- More examples of valid and invalid path patterns
// Example: Performance comparison let start = std::time::Instant::now(); let found_links = main_node.find(&["div", "a"]); println!("find() took: {:?}", start.elapsed()); let start = std::time::Instant::now(); let selected_links = doc.select("#main div a"); println!("select() took: {:?}", start.elapsed()); // Example: More path patterns main_node.find(&["div"]); // Direct children main_node.find(&["div", "*"]); // Any element under div main_node.find(&["div[class]", "a"]); // Elements with attributes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Examples.md
(1 hunks)README.md
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-wasm
- GitHub Check: test
🔇 Additional comments (1)
README.md (1)
666-778
: LGTM! Examples are consistent with Examples.md.The examples in README.md match those in Examples.md, providing consistent documentation across the repository.
Summary by CodeRabbit
Documentation
Chores