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

Small issue with it thinking that the file "4.2BSD-fortunes" is a percentage. #94

Closed
wants to merge 2 commits into from

Conversation

257m
Copy link

@257m 257m commented May 7, 2024

The current implementation checks if the first character is a digit and loops through the string to until it reaches a character that isn't. If the next character is a '.' after the digits it assumes the string is a non-integer and spits out an error. The problem with this approach is that just because it saw a '.' after a bunch of digits doesn't mean the user is even trying to use a percentage. In this case I am trying to specify a file. The update I made was that instead of that it goes to the end of the string and checks if the last character is a '%', if so it will it give an error after a '.' but if there isn't a '%', it will assume that it is not a percentage specifying a probability.

257m added 2 commits May 7, 2024 17:17
… since it had a \'.\' in it. The way I handled it is too loop through the entire string after the '.' and check for anything that isn't a digit. If so it will know it is not an attempt to use a non-integer percent value. I think a smarter way to handle this is to check if there is a \'%\' at the end of the string
…tead of the older naive approach. This should work fine.... Hopefully
@257m
Copy link
Author

257m commented May 7, 2024

Huh it seems like the Cmake files weren't in .gitignore, thats not good.

@shlomif
Copy link
Owner

shlomif commented Jun 30, 2024

Hi @257m ! Thanks for the pull-request but I am closing it given the commits in it contain large swaths of cmake files and codes. Feel free to submit a new one.

@shlomif shlomif closed this Jun 30, 2024
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