-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Adding 'each .. of' syntax #3179
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a good start. I've made a few comments that I hope are helpful.
packages/pug-code-gen/index.js
Outdated
+ '// iterate ' + each.obj + '\n' | ||
+ ';(function(){\n' | ||
+ ' var $$obj = ' + each.obj + ';\n' | ||
+ ' if (\'number\' == typeof $$obj.length) {'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to use the for (const ... of ...) {
js syntax and not refer to length
at all as in general iteration objects are not referenced by indexing. We'll need to decide how/if we want to handle the key
. One option would be to assume value, key
means de-structure as [value, key]
. I think it might be better to offer:
each x of foo
and
each [x, y] of foo
rather than supporting:
each x, y of foo
That way, if we wanted to, we'd have a clear path towards supporting more destructuring in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed it so you can have
each [key, value] of map
as well as
each keyValPair of map
@ForbesLindesay I've removed all 'else' syntax, and have added the ability to do each [key, val] of map Anything else? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking loads closer.
@ForbesLindesay I've updated the parser as well as the code-gen library. Anything else needing a change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is probably the last few comments.
@ForbesLindesay That's awesome to hear. Anything else that could be fixed up? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Would you mind following up with a couple of tests cases (in a separate PR)?
You can see https://github.com/pugjs/pug/tree/master/packages/pug/test/extends-not-top-level for a test that covers the error handling - i.e. you can test things like each foo, bar] of bing
and https://github.com/pugjs/pug/tree/master/packages/pug/test/shadowed-block gives you an example of testing a successful render. It would be good to have tests for Array, Set and Map.
This is now merged and will go in the next release. |
That's awesome to hear. Thanks for the help with getting me started. I'll post the PR now. |
Is it available now? |
pug (2.0.4 → 2.1.0)New Features
pug-code-gen (2.0.2 → 2.1.0)New Features
pug-lexer (4.1.0 → 5.0.0)Breaking Changes
pug-parser (5.0.1 → 6.0.0)Breaking Changes
pug-walk (1.1.8 → 1.2.0)New Features
Packages With No ChangesThe following packages have no user facing changes, so won't be released:
|
Accidentally found this feature. This feature is great, but does not mentioned in pug's iteration docs. |
This PR adds support for iterating over maps and other iterable objects with the JS syntax
each var of map
. This example patches issue #3170. The syntaxeach var of map
andfor var of map
has been introduced, buteach
andfor
can be used interchangeably, like with the original each syntax.All tests run and pass, but I haven't added any tests except for a small fix to recognise the
eachOf
function is thepug-lexer
.Example Code
JS
Pug
Output