-
Notifications
You must be signed in to change notification settings - Fork 235
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 blacklist support to simple-unless rule #518
Add blacklist support to simple-unless rule #518
Conversation
I can see how this is useful. Seems like a good feature to have. |
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.
Sorry for the delayed review here! This seems like a nice addition to me, I left a small inline tweak and we also need a rebase.
Should be good to land after that.
lib/rules/lint-simple-unless.js
Outdated
@@ -142,6 +147,14 @@ module.exports = class LintSimpleUnless extends Rule { | |||
this._logMessage(message, loc.line, loc.column, actual); | |||
} | |||
|
|||
if (blacklist.length > 0 && blacklist.indexOf(param.path.original) > -1) { // blacklist.includes(param.path.original) |
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.
I think we can use .includes
now (we dropped Node 4 support a little while back), mind swapping to that?
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.
Sounds good! Also updated a similar usage in the whitelist check above.
@mattbalmer - 👋 think you might have time to circle back to this one? Again, I'm sorry I let it linger so long after you initially submitted it... |
03f0644
to
7d66681
Compare
Hey, no worries and sorry for my own slow response! Your suggestion has been included, and the styles updated to match the new linter rules. Let me know if you spot any other changes you'd like! |
Thank you! |
A project I'm working on has need of a
blacklist
feature in conjunction with the 'simple-unless' rule. We would like to allow most cases for{{unless}}
, but prevent certain helpers from being used with them.Code changes include adding that blacklist support, as well as modifying
maxHelpers
to treat a value of-1
as 'no limit'. This is useful because otherwise, to use theblacklist
, one would need to specify an arbitrarily large number formaxHelpers
(as it would raise errors on helpers not in theblacklist
when the limit, defaulted to 0, was reached).