Skip to content
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

Handle Decimal Number Edge Case #10

Merged
merged 3 commits into from Jan 3, 2022

Conversation

Atixo
Copy link
Contributor

@Atixo Atixo commented Jan 3, 2022

Description

I noticed on the Hololive subreddit that pekofy-bot has an edge case where if a user pekofys a sentence that contains a decimal number, peko would be added in between the number as it counts as a match.

Issue

When a user uses !pekofy on a sentence that contains a decimal number, the sentence is improperly injected with a peko in the decimal number.

Example:
image

Potential Solution

Please let me know if the code below is satisfactory, I added a function just to check whether a match is a decimal number since I did not want to mess with the regex already in place. I've also only tested this using english phrases.

@emso-c
Copy link
Owner

emso-c commented Jan 3, 2022

Hello, I've added some unit tests (7954b3d) in case you want to check. Sorry for not implementing them earlier.

As for your code, just put the if statement in the try-catch block right below and it'll be perfect. Right now it might throw error since last_word might return empty.

Simply change

last_word = regex.search(r'[^\W_]', new_text[i::-1]) # find the nearest alphanumeric behind match point
if is_decimal_number(new_text, i, last_word.group()):
    continue
try:
    j = i - last_word.start() + 1 # index to insert keyword
...

to

last_word = regex.search(r'[^\W_]', new_text[i::-1]) # find the nearest alphanumeric behind match point
try:
    if is_decimal_number(new_text, i, last_word.group()):
        continue
    j = i - last_word.start() + 1 # index to insert keyword
...

I'll accept your PR as soon as you fix it. Thank you for your contribution!

@Atixo
Copy link
Contributor Author

Atixo commented Jan 3, 2022

Thank you for getting back to me so quickly! And I looked over the unit tests, they look great!

My fault on placing it outside the try, I did not realize that the match could have been empty. I've updated the PR accordingly!

@emso-c emso-c merged commit 4f691a6 into emso-c:main Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants