-
Notifications
You must be signed in to change notification settings - Fork 312
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
Add candidateFilters option #107
base: master
Are you sure you want to change the base?
Conversation
@luin @haroldtreen Please help review, and give some suggestions, thanks. |
This looks like an interesting option @hankliu62 👍 . What do you think about calling it Curious if @luin has thoughts because I know less about the history of this module 😅 . |
README.md
Outdated
@@ -81,6 +81,23 @@ read(url, { | |||
}); | |||
``` | |||
|
|||
- `candidatesFilters` which allow set your own filters for candidate tags. |
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.
This would read better as.
which allows you to set your own filters for candidate tags.
README.md
Outdated
read(url, { | ||
candidatesFilters: [ | ||
function (obj) { | ||
if (obj.tagName === 'ARTICLE' && elem.getAttribute('type') === 'video') { |
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.
Maybe could include a comment explaining what this filter does?
eg.
// ...
// Filter any article tags with a type of "video"
function(obj) {
// ...
}
README.md
Outdated
```javascript | ||
read(url, { | ||
candidatesFilters: [ | ||
function (obj) { |
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.
obj
doesn't describe what this function is passed. Maybe something like candidateNode
?
README.md
Outdated
read(url, { | ||
candidatesFilters: [ | ||
function (obj) { | ||
if (obj.tagName === 'ARTICLE' && elem.getAttribute('type') === 'video') { |
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.
Also think elem
is not defined in this example. I think you meant to make it the same as obj
?
@haroldtreen, thanks for all your help! I've updated PR according to your instructions. Let me know if I've missed something. |
Thanks for the updates @hankliu62 ! Only last thing I would consider is bumping the version in package.json to |
No description provided.