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

add '0x' handling for divide by 0 scenarios #36390

Merged
merged 2 commits into from
Jan 2, 2025

Conversation

plante-msft
Copy link
Contributor

@plante-msft plante-msft commented Dec 17, 2024

Summary of the Pull Request

Closes #36032. Adds divide by zero handling for hex numbers in the calculator plugin.

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Tested that reported scenario is fixed, as well as additional equations.

@plante-msft plante-msft marked this pull request as ready for review December 18, 2024 14:05
@jaimecbernardo jaimecbernardo added the Needs-Review This Pull Request awaits the review of a maintainer. label Dec 18, 2024
Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you!

@jaimecbernardo jaimecbernardo merged commit 2ba5fb7 into main Jan 2, 2025
16 checks passed
@dkaszews
Copy link

dkaszews commented Jan 2, 2025

@plante-msft @jaimecbernardo Won't this crash now on 1 / 0x0?

@dkaszews
Copy link

dkaszews commented Jan 2, 2025

Plus, checking division by zero with regex seems like a very silly way to do things, since it won't catch things like 1 / (1 - 1) or other arbitrary expressions in denominator.

@jaimecbernardo
Copy link
Collaborator

jaimecbernardo commented Jan 3, 2025

It will not crash, just show a different NaN (Not a Number) or other error instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Review This Pull Request awaits the review of a maintainer.
Projects
Development

Successfully merging this pull request may close these issues.

"Divide by zero" trying to by divide hex number in calculator
3 participants