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.

Sign up for GitHub

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

Jump to bottom

parser: allow numeric literals as object keys #223

Closed
wants to merge 2 commits into from
Closed

parser: allow numeric literals as object keys #223

wants to merge 2 commits into from

Conversation

belochub
Copy link
Member

@belochub belochub added this to the 1.0.0 milestone Jun 17, 2017
lundibundi
lundibundi approved these changes Jun 17, 2017
View reviewed changes
Copy link
Member

@lundibundi lundibundi left a comment

Choose a reason for hiding this comment

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

LGTM

@belochub
Copy link
Member Author

@nechaido, ping.

nechaido
nechaido requested changes Jun 19, 2017
View reviewed changes
Copy link
Member

@nechaido nechaido left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, but we need tests for this. I propose to add something like:

{
  name:'object with numeric keys',
  value: { 2: 3 },
  serialized: '{2:3}'
}

to /test/fixtures/serde-test-cases/serde.js

@belochub
Copy link
Member Author

@nechaido, adding the test you proposed to /test/fixtures/serde-test-cases/serde.js, will definitely break the build, this doesn't belong to regular test cases yet, in my opinion, it should be added to current json5 tests because it belongs to json5 specification and it isn't implemented in JavaScript parser yet, which makes it a "to do" test case for now.
I will add the corresponding test to json5 test cases if you don't mind.

@nechaido
Copy link
Member

@belochub you are right, go ahead.

@belochub
Copy link
Member Author

@nechaido, I've added the test, PTAL.

nechaido
nechaido approved these changes Jun 19, 2017
View reviewed changes
Copy link
Member

@nechaido nechaido left a comment

Choose a reason for hiding this comment

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

LGTM

aqrln pushed a commit that referenced this pull request Jun 20, 2017
@belochub @aqrln
parser: allow numeric literals as object keys
635ea15 
Refs: https://github.com/metarhia/jstp/issues/152
PR-URL: #223
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Dmytro Nechai <nechaido@gmail.com>
@aqrln
Copy link
Member

aqrln commented Jun 20, 2017

Landed in 635ea15.

@aqrln aqrln closed this Jun 20, 2017
@aqrln aqrln deleted the parser-allow-numeric-literals branch June 20, 2017 10:25
belochub added a commit that referenced this pull request Jan 22, 2018
@belochub
parser: allow numeric literals as object keys
7f44984 
Refs: https://github.com/metarhia/jstp/issues/152
PR-URL: #223
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Dmytro Nechai <nechaido@gmail.com>
belochub added a commit that referenced this pull request Jan 22, 2018
@belochub
parser: allow numeric literals as object keys
4504994 
Refs: https://github.com/metarhia/jstp/issues/152
PR-URL: #223
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Dmytro Nechai <nechaido@gmail.com>
@belochub belochub mentioned this pull request Jan 22, 2018
Merged
belochub added a commit to metarhia/mdsf that referenced this pull request Jul 19, 2018
@belochub @aqrln
parser: allow numeric literals as object keys
62a4e1e 
Refs: https://github.com/metarhia/jstp/issues/152
PR-URL: metarhia/jstp#223
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Dmytro Nechai <nechaido@gmail.com>
belochub added a commit to metarhia/mdsf that referenced this pull request Jul 19, 2018
@belochub @aqrln
parser: allow numeric literals as object keys
7f2ee95 
Refs: https://github.com/metarhia/jstp/issues/152
PR-URL: metarhia/jstp#223
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Dmytro Nechai <nechaido@gmail.com>
belochub added a commit to metarhia/mdsf that referenced this pull request Jul 21, 2018
@belochub
parser: allow numeric literals as object keys
bdff719 
Refs: https://github.com/metarhia/jstp/issues/152
PR-URL: metarhia/jstp#223
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Dmytro Nechai <nechaido@gmail.com>
@aqrln aqrln mentioned this pull request Feb 12, 2019
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aqrln aqrln aqrln approved these changes

@lundibundi lundibundi lundibundi approved these changes

@nechaido nechaido nechaido approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@belochub @nechaido @aqrln @lundibundi