Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
For each index & value loop. #6562
For each index & value loop. #6562
Changes from 3 commits
3cd421e
ba3be6f
36c1e09
8c61221
1000200
9ca6aa5
edf1230
ebb6365
7f8d74a
0eb2802
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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'm not sure about this
=
syntax. Is there a reason you chose it?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 it was either from one of the issues/discussions suggesting this or I was copying from some other language, It's been about six months and I can't really remember.
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.
Oh I remember, I think there was a worry about
{x} and {y}
being confusingly-ambiguous (not in terms of parsing but for a user to read, maybe thinking it was looping two items at a time) between a regular list of expressions and this specific syntax where theand
is literally part of it, and we wanted a less-ambiguous alternative that emphasised the keys were mapped to the values.I'd rather it wasn't a symbol (=) although I don't mind it, but I couldn't think of a better single word for it.
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 we should avoid symbols if possible. I don't think
for each {_a} = {_b} in {_c::*}
is great.In fact, we might want to require the usage of the keywords
key|index
andvalue
for this option.for each {_a} and {_b} in {_c::*}
makes it seem likec
is holding some sort of tuple.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'm fine with getting rid of =, how would you feel about
,
as an alternative? Python has e.g.for key, value in dict.items()
and that's not terrible IMO.I'm just worried that writing out the whole
for key {blah} and value {blah} in ...
is going to be more verbose than is necessary, when really we just need tome way to indicate one is the index representing the other.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.
That would certainly be a better shorthand than
=
. I am okay with it, though I dont thinkfor {_key} and {_value} in ...
is too badThere 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.
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.
Just to bring it in line with the
keyStore
check (and I think this is clearer)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.
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.
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.
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.
The parentheses use seems a bit odd here