Make ignored characters configurable #24

Merged
martinhrvn merged 5 commits from remove-special-characters into master 2021-12-30 12:18:25 -05:00
martinhrvn commented 2021-12-20 06:12:16 -05:00 (Migrated from github.com)

I added the ability to configure the special characters to be replaced. I also changed the implementation to be a loop over the string. Let me know if you disagree with this.

The mapping could also be implemented as map[string]string however this would complicate things with remembering the indexes. Currently there is only the case of () that has special handling.

I added the ability to configure the special characters to be replaced. I also changed the implementation to be a loop over the string. Let me know if you disagree with this. The mapping could also be implemented as `map[string]string` however this would complicate things with remembering the indexes. Currently there is only the case of `()` that has special handling.
codecov-commenter commented 2021-12-20 15:41:11 -05:00 (Migrated from github.com)

Codecov Report

Merging #24 (d091996) into master (58c7739) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #24   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         2    +1     
  Lines          134       137    +3     
=========================================
+ Hits           134       137    +3     
Impacted Files Coverage Δ
goaway.go 100.00% <100.00%> (ø)
specialcharacters.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7753940...d091996. Read the comment docs.

# [Codecov](https://codecov.io/gh/TwiN/go-away/pull/24?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.) Report > Merging [#24](https://codecov.io/gh/TwiN/go-away/pull/24?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.) (d091996) into [master](https://codecov.io/gh/TwiN/go-away/commit/58c77392f42ae745dd64db707b474359eb1d8948?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.) (58c7739) will **not change** coverage. > The diff coverage is `100.00%`. [![Impacted file tree graph](https://codecov.io/gh/TwiN/go-away/pull/24/graphs/tree.svg?width=650&height=150&src=pr&token=25T9OQT9TM&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.)](https://codecov.io/gh/TwiN/go-away/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.) ```diff @@ Coverage Diff @@ ## master #24 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 1 2 +1 Lines 134 137 +3 ========================================= + Hits 134 137 +3 ``` | [Impacted Files](https://codecov.io/gh/TwiN/go-away/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.) | Coverage Δ | | |---|---|---| | [goaway.go](https://codecov.io/gh/TwiN/go-away/pull/24/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.#diff-Z29hd2F5Lmdv) | `100.00% <100.00%> (ø)` | | | [specialcharacters.go](https://codecov.io/gh/TwiN/go-away/pull/24/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.#diff-c3BlY2lhbGNoYXJhY3RlcnMuZ28=) | `100.00% <100.00%> (ø)` | | ------ [Continue to review full report at Codecov](https://codecov.io/gh/TwiN/go-away/pull/24?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.) > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/TwiN/go-away/pull/24?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.). Last update [7753940...d091996](https://codecov.io/gh/TwiN/go-away/pull/24?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.).
TwiN (Migrated from github.com) reviewed 2021-12-21 18:04:11 -05:00
@ -0,0 +1,26 @@
package goaway
TwiN (Migrated from github.com) commented 2021-12-21 18:04:10 -05:00

Does it really make sense to name them replacements, given that they're technically skipped/ignored/trimmed?

Does it really make sense to name them replacements, given that they're technically skipped/ignored/trimmed?
martinhrvn (Migrated from github.com) reviewed 2021-12-22 04:10:19 -05:00
@ -0,0 +1,26 @@
package goaway
martinhrvn (Migrated from github.com) commented 2021-12-22 04:10:19 -05:00

good catch. I renamed and removed unnecessary variable

good catch. I renamed and removed unnecessary variable
TwiN (Migrated from github.com) requested changes 2021-12-22 19:33:38 -05:00
@ -43,6 +45,8 @@ func NewProfanityDetector() *ProfanityDetector {
profanities: DefaultProfanities,
TwiN (Migrated from github.com) commented 2021-12-22 19:33:34 -05:00

Can you rename specialCharactersReplacementMap to ignoredCharacters and DefaultIgnoredSpecialCharacters to DefaultIgnoredCharacters?

WithSpecialCharacters should also be renamed to WithIgnoredCharacters, and the README should also be updated to reflect the new feature 😅

Can you rename `specialCharactersReplacementMap` to `ignoredCharacters` and `DefaultIgnoredSpecialCharacters` to `DefaultIgnoredCharacters`? `WithSpecialCharacters` should also be renamed to `WithIgnoredCharacters`, and the README should also be updated to reflect the new feature 😅
TwiN (Migrated from github.com) reviewed 2021-12-29 23:49:26 -05:00
@ -43,6 +45,8 @@ func NewProfanityDetector() *ProfanityDetector {
profanities: DefaultProfanities,
TwiN (Migrated from github.com) commented 2021-12-29 23:49:26 -05:00

@martinhrvn I see you re-requested my review, but despite 👍ing my comment, but you didn't push any changes 😅

@martinhrvn I see you re-requested my review, but despite 👍ing my comment, but you didn't push any changes 😅
martinhrvn (Migrated from github.com) reviewed 2021-12-30 04:06:43 -05:00
@ -43,6 +45,8 @@ func NewProfanityDetector() *ProfanityDetector {
profanities: DefaultProfanities,
martinhrvn (Migrated from github.com) commented 2021-12-30 04:06:42 -05:00

@TwiN it must've got stuck in a pipe somewhere 🤦 I pushed now

@TwiN it must've got stuck in a pipe somewhere 🤦 I pushed now
TwiN (Migrated from github.com) approved these changes 2021-12-30 12:17:44 -05:00
@ -65,6 +65,13 @@ falseNegatives := []string{"dumbass"}
profanityDetector := goaway.NewProfanityDetector().WithCustomDictionary(profanities, falsePositives, falseNegatives)
TwiN (Migrated from github.com) commented 2021-12-30 12:17:06 -05:00
By default, the characters `.` `_` `-` `?` `(` `)` `|` `~` are stripped from the original string. 
```suggestion By default, the characters `.` `_` `-` `?` `(` `)` `|` `~` are stripped from the original string. ```
TwiN commented 2021-12-30 12:18:29 -05:00 (Migrated from github.com)

Awesome work @martinhrvn! Thank you for the contribution :)

Awesome work @martinhrvn! Thank you for the contribution :)
TwiN commented 2021-12-30 15:44:54 -05:00 (Migrated from github.com)

FYI, I merged ignoredCharacters and leetSpeakReplacementMap into one. See 803cf25f73

FYI, I merged `ignoredCharacters` and `leetSpeakReplacementMap` into one. See 803cf25f73f1b18ce3ae4705caf9411590f56160
martinhrvn commented 2021-12-30 18:20:41 -05:00 (Migrated from github.com)

FYI, I merged ignoredCharacters and leetSpeakReplacementMap into one. See 803cf25

I was actually thinking about that and I think it makes sense to do.

> FYI, I merged `ignoredCharacters` and `leetSpeakReplacementMap` into one. See [803cf25](https://github.com/TwiN/go-away/commit/803cf25f73f1b18ce3ae4705caf9411590f56160) I was actually thinking about that and I think it makes sense to do.
TwiN commented 2021-12-30 18:27:56 -05:00 (Migrated from github.com)

Awesome :p

I just released the feature in v1.6.0

Awesome :p I just released the feature in v1.6.0
martinhrvn commented 2021-12-31 03:41:41 -05:00 (Migrated from github.com)

I was just thinking in that case we could probably get rid of the boolean configs for special characters and leet speak, since we could just set the appropriate replacements and use those. It would simplify the logic of the sanitization.. I will create PR and let you decide how you feel about that

I was just thinking in that case we could probably get rid of the boolean configs for special characters and leet speak, since we could just set the appropriate replacements and use those. It would simplify the logic of the sanitization.. I will create PR and let you decide how you feel about that
TwiN commented 2021-12-31 08:55:47 -05:00 (Migrated from github.com)

Yeah but that's a breaking change, and I'm not too keen on making a breaking change at the moment

Yeah but that's a breaking change, and I'm not too keen on making a breaking change at the moment
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
TwiN/go-away!24
No description provided.