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

[dice] Case insensitivity and better error handling for negative numbers. #969

Merged
merged 3 commits into from
Dec 12, 2015

Conversation

Ganon11
Copy link
Contributor

@Ganon11 Ganon11 commented Dec 10, 2015

Case insensitivity

Instead of matching for the literal d or v, match [dD] and [vV] appropriately.

Negative numbers

Match an optional - character before each possible number, and handle negative inputs in the _roll_dice method. There was already a check for negative dice_type, but because the - wasn't being captured, this code wasn't being used. I added a check for dice_num < 0 (which returns None) and drop < 0 (which ignores the drop_lowest value entirely).

Partially addresses #874

First, add case-insensitivity to the "d" and "v" letters in the input
regex by capturing [dD] and [vV] in regexes.

Next, make sure we can handle negative numbers properly by capturing
a possible "-" in front of any numbers. If dice_num or dice_type ends
up negative, abort _roll_dice early (dice_type already had a negative
check that wasn't being hit because "-" wasn't captured).
First, add case-insensitivity to the "d" and "v" letters in the input
regex by capturing [dD] and [vV] in regexes.

Next, make sure we can handle negative numbers properly by capturing
a possible "-" in front of any numbers. If dice_num or dice_type ends
up negative, abort _roll_dice early (dice_type already had a negative
check that wasn't being hit because "-" wasn't captured).
embolalia added a commit that referenced this pull request Dec 12, 2015
[dice] Case insensitivity and better error handling for negative numbers.
@embolalia embolalia merged commit 047a99e into sopel-irc:master Dec 12, 2015
@embolalia
Copy link
Contributor

Thanks!

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.

2 participants