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 built in detection #121

Merged
merged 1 commit into from
May 6, 2020
Merged

Fix built in detection #121

merged 1 commit into from
May 6, 2020

Conversation

brummelte
Copy link
Contributor

@brummelte brummelte commented May 5, 2020

Variable names starting with a built in, will be detected as a built in.
So this does not work: +++formatNumber(123)+++. It is detected to use FOR.

notBuiltIns before change:

new RegExp(`^FOR`).test("FOR") // true
new RegExp(`^FOR`).test("FOR ") // true
new RegExp(`^FOR`).test("FOR\n") // true
new RegExp(`^FOR`).test("FORMAT\n") // true
new RegExp(`^FOR`).test("OTHER\n") // false

notBuiltIns after change:

new RegExp(`^FOR\\b`).test("FOR") // true
new RegExp(`^FOR\\b`).test("FOR ") // true
new RegExp(`^FOR\\b`).test("FOR\n") // true
new RegExp(`^FOR\\b`).test("FORMAT\n") // false
new RegExp(`^FOR\\b`).test("OTHER\n") // false

Allow variable names to start with a built in
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 92.559% when pulling 756b9e7 on brummelte:patch-1 into eb0c8cf on guigrpa:master.

@jjhbw jjhbw changed the base branch from master to next May 6, 2020 15:22
@jjhbw
Copy link
Collaborator

jjhbw commented May 6, 2020

Awesome, thanks for noticing. I'll merge this in the staging branch and add some test cases for this behaviour.

@jjhbw jjhbw merged commit bd9afd4 into guigrpa:next May 6, 2020
jjhbw added a commit that referenced this pull request May 6, 2020
jjhbw pushed a commit that referenced this pull request May 12, 2020
Allow variable names to start with a built-in command
jjhbw added a commit that referenced this pull request May 12, 2020
jjhbw pushed a commit that referenced this pull request May 17, 2020
Allow variable names to start with a built-in command
jjhbw added a commit that referenced this pull request May 17, 2020
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.

None yet

3 participants