Skip to content

Conversation

rtakacs
Copy link
Contributor

@rtakacs rtakacs commented Jul 29, 2015

Modified some places to support unicode characters.
Also renamed some variables to be more clearer.

@rtakacs
Copy link
Contributor Author

rtakacs commented Jul 29, 2015

Solution for #406

@rtakacs rtakacs force-pushed the json_stringify_unicode branch from 9582c1c to 1419e37 Compare July 29, 2015 12:36
@egavrin egavrin added this to the ECMA builtins milestone Jul 29, 2015
@egavrin egavrin added bug Undesired behaviour ecma builtins Related to ECMA built-in routines labels Jul 29, 2015
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this is length not size?

@rtakacs rtakacs force-pushed the json_stringify_unicode branch from 1419e37 to d08c5ec Compare July 31, 2015 09:15
@zherczeg
Copy link
Member

zherczeg commented Aug 3, 2015

LGTM

@dbatyai dbatyai assigned ruben-ayrapetyan and unassigned zherczeg Aug 4, 2015
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need space_buff?
As I see, we can just call ecma_new_ecma_string_from_utf8 (string_buff, space_buff_size).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm... You are right, we don't need space_buff.
Updating.

@sand1k sand1k assigned rtakacs and unassigned sand1k Aug 5, 2015
@rtakacs rtakacs force-pushed the json_stringify_unicode branch from d08c5ec to fe67b75 Compare August 5, 2015 13:02
Copy link
Contributor

Choose a reason for hiding this comment

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

To extract a substirng, it is better to use:
context_p.gap_str_p = ecma_string_substr (space_str_p, 0, 10).

Sorry, noticed just now, that this fragment could be simplified so much.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice! FIxed :)

@rtakacs rtakacs force-pushed the json_stringify_unicode branch 2 times, most recently from a83cd9d to 5f296ba Compare August 5, 2015 13:32
@sand1k
Copy link
Contributor

sand1k commented Aug 5, 2015

LGTM.

@rtakacs rtakacs force-pushed the json_stringify_unicode branch 2 times, most recently from b79e960 to 4ac18b8 Compare August 6, 2015 11:09
@rtakacs rtakacs force-pushed the json_stringify_unicode branch from 4ac18b8 to 5888401 Compare August 6, 2015 11:16
@kkristof kkristof merged commit 5888401 into jerryscript-project:master Aug 6, 2015
@rtakacs rtakacs deleted the json_stringify_unicode branch January 18, 2019 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Undesired behaviour ecma builtins Related to ECMA built-in routines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants