Skip to content
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 read filter option #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jamesspencer
Copy link

Inspired by issue #2 #2

@ProLoser
Copy link
Contributor

@jamesspencer I'm curious what the filter function you're using is.

@jamesspencer
Copy link
Author

We needed to strip any leading slashes from script sources before passing the results directly to another 3rd party task:

{
  selector: 'script[src]',
  attribute: 'src',
  writeto: 'myJsRefs',
  isPath: true,
  filter: function(val) {
    return val.replace(/^\//, '');
  }
}

Looking back, filter is the wrong term to use as this is about modifying the data rather than determining whether it should be added to the collection.

@ProLoser
Copy link
Contributor

Yeah. I would just change the property to callback. You also could improve performance by simply doing an if-set check instead of declaring a noop.

@ProLoser
Copy link
Contributor

Actually. It would make sense if it worked as both a cleanup AND filter. Such as if you return 'false' it doesn't get added to the collection.

@jamesspencer
Copy link
Author

Changing to callback sounds good, also performance note.

RE filtering: this did cross my mind but by allowing the user to manipulate the data also allows then to return non-string values; should their intent be to collect a list of booleans (based on some criteria) then this functionality would strip results. A more flexible and explicit design would have separate callback and filter operations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants