Skip to content
This repository has been archived by the owner on Dec 19, 2020. It is now read-only.

Can't reconfigure after first blacklist configuration #8

Open
chrisvfritz opened this issue Jun 24, 2013 · 6 comments
Open

Can't reconfigure after first blacklist configuration #8

chrisvfritz opened this issue Jun 24, 2013 · 6 comments

Comments

@chrisvfritz
Copy link

If I try to reconfigure obscenity to switch between multiple blacklists, it'll claim to be pointing to the new list, without actually using it in sanitize. For example, in this code:

class String
    def sex_filter
        Obscenity.config.blacklist = File.join(Rails.root,"/config/filters/sex.yml")
        Obscenity.replacement(:stars).sanitize(self)
    end

    def profanity_filter
        Obscenity.config.blacklist = File.join(Rails.root,"/config/filters/profanity.yml")
        Obscenity.replacement(:stars).sanitize(self)
    end

    def hate_filter
        Obscenity.config.blacklist = File.join(Rails.root,"/config/filters/hate.yml")
        Obscenity.replacement(:stars).sanitize(self)
    end

    def violence_filter
        Obscenity.config.blacklist = File.join(Rails.root,"/config/filters/violence.yml")
        Obscenity.replacement(:stars).sanitize(self)
    end
end

Whichever filter I call first sets the list that all other filters will use until I restart my Rails app, even though I'm telling it to point to another list. If I call Obscenity.config.blacklist, it'll even report to be pointing to the correct list.

@chrisvfritz
Copy link
Author

I figured out a way to work around this. The solution is don't use Obscenity.config.blacklist to set the blacklist. Instead, use Obscenity::Base.blacklist. It doesn't seem like a very clean solution to me, but it works for now.

I haven't tested this, but a better fix might be to change this method in the Config class:

def blacklist=(value)
  @blacklist = value == :default ? DEFAULT_BLACKLIST : value
end

...to this...

def blacklist=(value)
  Obscenity::Base.blacklist = @blacklist = value == :default ? DEFAULT_BLACKLIST : value
end

@chesterbr
Copy link
Contributor

What about keeping the issue open, @chrisvfritz? It is reasonable to expect your earlier code to work...

@tjackiw
Copy link
Owner

tjackiw commented Jul 2, 2013

Yes, please keep the issue open.

Thanks.
On Jul 2, 2013 1:49 PM, "Carlos Duarte do Nascimento (Chester)" <
[email protected]> wrote:

What about keeping the issue open, @chrisvfritzhttps://github.com/chrisvfritz?
It is reasonable to expect your earlier code to work...


Reply to this email directly or view it on GitHubhttps://github.com//issues/8#issuecomment-20375161
.

@chrisvfritz chrisvfritz reopened this Jul 2, 2013
@chrisvfritz
Copy link
Author

This goes beyond the scope of the bug, but my preferred solution would involve the creation of Obscenity instances, each of which would have their own configuration, and to do away with the config class altogether. So for example, I could do:

sex_filter = Obscenity.new(blacklist: ['sexual','language'], replacement: :stars)
# then should I want to later modify the sex_filter, I could simply use... 
sex_filter.blacklist = ['different','words']
sex_filter.whitelist = ['please','thank you']
sex_filter.replacement = :vowels
# and using the filter on text would work very much the same
sex_filter.profane? "here's some text to check against the blacklist"
sex_filter.sanitize "some more text"

That makes more intuitive sense to me, but maybe this gem is simply focused on a different set of needs. This change would involve a pretty substantial rewrite and I could give it a shot, but I'm not sure if it would be an appropriate pull request, as it would break the gem for others using it. Thoughts?

@chrisvfritz
Copy link
Author

OK - I ended up just writing a new language_filter gem that did everything I needed it to, as I needed one relatively urgently for work. The development was too rushed to be test-driven unfortunately, so tests will be coming soon. But here are some advantages over the Obscenity gem:

  • The ability to create multiple, independently configured filter instances
  • Simpler configuration
  • Multiple pre-prackaged matchlists (i.e. blacklists) for language type discrimination (hate, profanity, sex, and violence)
  • More robust exceptionlist (i.e. whitelist) handling
  • And more in the gem's README...

I'd love to hear what others think and to get contributions to make it better. There's plenty of room for improvement of the matchlists - and for more matchlist categories to be added. And there still may be a bug or two lurking around. And of course, if anyone wanted to write the tests while I'm away for the fourth, I would welcome the pull request. :-)

@jswencki
Copy link

jswencki commented Feb 7, 2014

Any feedback on whether this or the other PR's are going to be merged? Ran into same issue. Thanks!

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

No branches or pull requests

4 participants