search

es128 / progeny

:baby: Recursively finds dependencies of style and template source files
MIT License
29 stars 15 forks source link

Allowing specification of an exclusion list #3

Closed ghost closed 10 years ago

ghost commented 10 years ago

Added tests and functionality for being able to provide one or more regular expressions to exclude against.

es128 commented 10 years ago

This appears to include a couple of breaking changes. First, the arbitrary change from exclusion to excludes would break this for anyone who specified a custom configuration including that property. What was the reason for that change? Secondly, the method chosen for array detection breaks the ability to specify a plain string while there are other viable methods that wouldn't have this side-effect.

Beyond that, what is your use case motivating this change which can't easily be done with a single regex?

I wouldn't necessarily mind allowing an array, but I would need to see a cleaner implementation.

ghost commented 10 years ago

sorry, no particular reason for that change other than the nomenclature. i overlooked that it would break previous use of the function.

the use case in mind was to provide a cleaner way of providing a dynamic list of exclusions, specifically, allowing users to define language specific exclusions over different configuration files. thinking about it now, the overhead of just aggregating those on our end and joining them into one regexp, while not ideal, is probably less than having to change the progeny library itself.

in the end, its probably more of a cosmetic change to avoid super long regular expressions, but that is probably up to you as to whether or not that makes sense.

es128 commented 10 years ago

I would accept a PR that enabled the array functionality cleanly. Looks like it could be done pretty simply with just https://github.com/es128/progeny/pull/3/files#diff-2fbc5085a7bbe6fb696a1a3b75a6f420R37 and a proper array check at the prior line. Then even an array of strings could be used, for the very regex-phobic.

ghost commented 10 years ago

got it. will update accordingly.