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

Group Negative Digits #445

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

atummons
Copy link

This changes from isDigit (only finds positive numbers) to casting to int to find all numbers. If an int is found, it converts it back to a string and strips the negative sign from it. It will then group as normal and replace the number portion with the grouped digits and leave the negative sign intact.

This changes from isDigit (only finds positive numbers) to casting to int to find all numbers. If an int is found, it converts it back to a string and strips the negative sign from it.  It will then group as normal and replace the number portion with the grouped digits and leave the negative sign intact.
@atummons
Copy link
Author

Not sure what the issue is here that failed, as it's a .C file that I did not touch. Also, I cannot run any of the tests on my local (Windows problems?) I didn't see where any of the tests were actually run in this build either. If the tests ran, great, if not, I would appreciate someone running them to make sure they all pass before merging. Thanks!

Tied to this issue reporting on the MPF side: missionpinball/mpf#1683

@toomanybrians
Copy link
Member

The Windows cloud builds are broken currently. (Well, they were last time I checked a few months ago.) I was hoping a windows guru would figure out why, but no luck. (I'm a Mac user with a bit of Linux, I don't use Windows anymore.)

I do have a Windows VM and I was able to get the builds to run fine locally, but they just randomly stopped working on GitHub. I'm going to look at the MPF installers again next week (specifically the MC installer, as MPF is fine), so I'll see if I can get them fixed and then merge this then.

@atummons
Copy link
Author

Thanks, Brian. And no rush on anything, saw someone reported the issue and was something I could fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants