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

Explicit keywords #57

Merged
merged 5 commits into from
Jul 21, 2017
Merged

Explicit keywords #57

merged 5 commits into from
Jul 21, 2017

Conversation

tjvr
Copy link
Collaborator

@tjvr tjvr commented Jul 19, 2017

Some thoughts on #53.

See the updated readme.

@tjvr
Copy link
Collaborator Author

tjvr commented Jul 19, 2017

I think making the keyword behaviour explicit is probably a good idea.

I'm not sure about this API, though. Perhaps it should be a nested object?

moo.compile({
  identifier: {match: /[a-zA-Z]+/, keywords: {keyword: ['moo', 'cow']}},
})

Or with individual types:

moo.compile({
  identifier: {
    match: /[a-zA-Z]+/,
    keywords: {
        'kw-class': 'class',
        'kw-def': 'def',
        'kw-if': 'if',
    },
  },
})

@nathan
Copy link
Collaborator

nathan commented Jul 19, 2017

Or with individual types:

itt(['class', 'def', 'if'])
.map(k => ['kw-' + k, k])
.toObject()

(Iterator adaptors solve every* problem!)

moo.js Outdated
(byLength[keyword.length] = byLength[keyword.length] || []).push(keyword)
})

// fast string lookup
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you profiled this as being faster than a plain old switch on the value?

Copy link
Collaborator Author

@tjvr tjvr Jul 19, 2017

Choose a reason for hiding this comment

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

Not personally, but I've read that switching on the length first is faster.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tjvr
Copy link
Collaborator Author

tjvr commented Jul 19, 2017

Iterator adaptors solve every* problem!

Nice! :-)

@tjvr
Copy link
Collaborator Author

tjvr commented Jul 19, 2017

      keywords: {
        'kw-class': 'class',

I'm also not convinced having the object "backwards" like this, for consistency with the outer moo.compile, is correct.

@tjvr
Copy link
Collaborator Author

tjvr commented Jul 21, 2017

Let's do this.

@tjvr tjvr merged commit de59a33 into master Jul 21, 2017
tjvr added a commit that referenced this pull request Jul 21, 2017
@tjvr tjvr deleted the less-magic branch July 21, 2017 19:17
This was referenced Jul 21, 2017
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

2 participants