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

Replace itt example with modern js example #147

Merged
merged 1 commit into from
Oct 19, 2020
Merged

Replace itt example with modern js example #147

merged 1 commit into from
Oct 19, 2020

Conversation

AlansCodeLog
Copy link
Contributor

I noticed the link to the itt package in the readme doesn't work. It's not wrong though, the author just seems to have deleted the repo from github. Not sure what happened, but mapping an array to an object can be easily done with modern js without adding dependencies, so I just replaced the example.

@nathan
Copy link
Collaborator

nathan commented Oct 7, 2020

You replaced the wrong example; your code should replace the one under "Keyword Types," not the one under "Iteration." You could also just change the itt link to npm.im/itt.

If you want to replace the iteration example with modern JS, you'd have to do something like this:

function* ahead(n, xs) {
  const it = xs[Symbol.iterator]()
  let value, done, buf = []
  while (buf.length < n && ({value, done} = it.next()) && !done) {
    buf.push(value)
  }
  while (!done && ({value, done} = it.next()), !done ? buf.push(value) : buf.length) {
    yield buf
    buf = buf.slice(1)
  }
}
for (const [here, next] of ahead(1, lexer)) ...

which is pretty much just a golfier version of the source code for lookahead().

There's actually a typo in the iteration example anyway (it should be of, not =).

Also IMHO "modern javascript" sounds pretentious; you can just say

Object.fromEntries can help you construct repetitive keyword objects:

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

and anyone with a head on her shoulders can translate that into reduce if she wants to support older environments.

@AlansCodeLog
Copy link
Contributor Author

Oops, sorry about that! Must of searched for itt and pasted into the wrong place. I meant to replace the keyword example. I've updated the commit with your suggestions. Also updated the remaining itt link which I did not see was there and fixed the equal sign in that example. I think in that case where the alternative is not so short, relying on a package is not a bad idea.

Any reason why you removed it from github? It's just it gives the feeling of the package being unmaintained (given the npm page still links to the non-existent repo).

Copy link
Collaborator

@tjvr tjvr left a comment

Choose a reason for hiding this comment

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

Thanks both!

@tjvr tjvr merged commit a8dd439 into no-context:master Oct 19, 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