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

Fix infinite loop #49

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

Fix infinite loop #49

wants to merge 2 commits into from

Conversation

seedofjoy
Copy link

@seedofjoy seedofjoy commented Dec 3, 2016

Fixes infinite loop in case when number or letter appears outside of parentheses.

/some-regex/.test(undefined) always returns true

For example:

  • (add 1 2)5
  • (add 2 3)abs

Fixes infinite loop in case when number or letter appears outside of parentheses.

For example:
`(add 1 2)5` or `(add 2 3)abs`
@jamiebuilds
Copy link
Owner

Wait, /some-regex/.test(undefined) doesn't always return true? It returns false.

@seedofjoy
Copy link
Author

seedofjoy commented Dec 3, 2016

Sorry, it returns true only in LETTERS regex (but I think we should leave this check also in NUMBERS).

Try this example: /[a-z]/.test(undefined);. It returns true.

That's because undefined casting to string in this case.

@@ -423,7 +423,7 @@ function tokenizer(input) {
// Then we're going to loop through each character in the sequence until
// we encounter a character that is not a number, pushing each character
// that is a number to our `value` and incrementing `current` as we go.
while (NUMBERS.test(char)) {
while (char && NUMBERS.test(char)) {
Copy link

Choose a reason for hiding this comment

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

Maybe while(current < input.length && ... ) ?

@stardustman
Copy link

may be stupid advice, in the while loop of function tokenizer, there is a let variable named char, but char is a keyword in other language,like c or some others. may be chose another name.

@jinggk
Copy link

jinggk commented Jul 2, 2019

it's only a demo, isn't it?

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.

5 participants