fixes ruby behavior on added strings with capitals #7
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The previous implementation didn't do any lowercasing on the word list, so added words containing capital letters were never filtered for.
|
@dariusk Just thought I'd ping you! Any changes I should make? |
|
Oh, I missed this! Yeah, seems great, thanks for the fix. |
dariusk
added a commit
that referenced
this pull request
Nov 24, 2014
fixes ruby behavior on added strings with capitals
|
Did you by any chance upload this to rubygems or similar? Just checking, in case it needs to be updated somewhere. |
|
I haven't, but I'm not the original implementer. I didn't find a On Mon, Nov 24, 2014 at 11:10 AM, Darius Kazemi notifications@github.com
|
|
Cool, just checking. Thanks! |
mambocab
added a commit
to mambocab/wordfilter
that referenced
this pull request
Dec 21, 2014
fixes the bug from dariusk#7
fitnr
pushed a commit
to fitnr/wordfilter
that referenced
this pull request
Dec 30, 2014
adds & tests lazy normalization to list-checking fixes the bug from dariusk#7 makes add_words accept arbitrary iterables makes add_words accept a string adds convenience function to module exposes module utilitites fixes basestring spelling fixes data reading fixes syntax error fixes blacklist extension
fitnr
pushed a commit
to fitnr/wordfilter
that referenced
this pull request
Feb 10, 2015
adds & tests lazy normalization to list-checking fixes the bug from dariusk#7 makes add_words accept arbitrary iterables makes add_words accept a string adds convenience function to module exposes module utilitites fixes basestring spelling fixes data reading fixes syntax error fixes blacklist extension
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous implementation didn't do any downcasing on the word list, so added words containing capital letters were never filtered for, so if you added the string
CLEAN:then
Wordfilter::blacklisted?("this string was clean!")would return false, because it containsclean, notCLEAN. The JavaScript implementation doesn't seem to have this problem, I think because of the behavior oftoLowerCase.This implementation downcases the contents of the blacklist lazily, just before it's consulted. In principle, I'm pretty sure that's the right way to do it, but I have no idea if this implementation is good Ruby -- it may belong in
init_first_timeor equivalent. Seems to work, though!