Skip to content

Add json parse and stringify function to jerryscript c api #2243

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

Merged
merged 1 commit into from
Apr 5, 2018

Conversation

ZsoltRaduska
Copy link
Contributor

No description provided.

@@ -522,6 +522,8 @@ jerry_length_t jerry_get_typedarray_length (jerry_value_t value);
jerry_value_t jerry_get_typedarray_buffer (jerry_value_t value,
jerry_length_t *byte_offset,
jerry_length_t *byte_length);
jerry_value_t jerry_json_parse (const char * json_string);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be more consistent with the rest of the API if this would take a jerry_value_t representing the string.

@@ -522,6 +522,8 @@ jerry_length_t jerry_get_typedarray_length (jerry_value_t value);
jerry_value_t jerry_get_typedarray_buffer (jerry_value_t value,
jerry_length_t *byte_offset,
jerry_length_t *byte_length);
jerry_value_t jerry_json_parse (const char * json_string);
jerry_value_t jerry_json_stringfy (const jerry_value_t object_to_stringify);
Copy link
Contributor

Choose a reason for hiding this comment

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

JSON.parse and JSON.stringify take a couple of optional arguments.
I guess they could be exposed as different C APIs (i.e. jerry_json_parse_with_reviver, etc)

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify

@ZsoltRaduska ZsoltRaduska force-pushed the extendCapi branch 2 times, most recently from 82fa30c to 68cb0b5 Compare March 19, 2018 14:59
@@ -1109,7 +1109,7 @@ main (void)
cesu8_length = jerry_get_string_length (args[0]);
cesu8_sz = jerry_get_string_size (args[0]);

char string_console[cesu8_sz];
char string_console[7];
jerry_string_to_char_buffer (args[0], (jerry_char_t *) string_console, cesu8_sz);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rewrite this to the original value (do not use magic constants).

@@ -5561,3 +5561,66 @@ jerry_value_t jerry_get_typedarray_buffer (jerry_value_t value,
**See also**

- [jerry_create_typedarray](#jerry_create_typedarray)


Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary new line

jerry_release_value (parsed_json);
}
```
## jerry_stringify
Copy link
Contributor

Choose a reason for hiding this comment

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

add a new line here before the heading

## jerry_stringify

**Summary**
Stringify a jerry_value_t object
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a new line here after the emphasized text


```c
jerry_value_t jerry_json_parse (const char * json_string)
```
Copy link
Contributor

Choose a reason for hiding this comment

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

remove space after the asterisk. (const char *json_string)

jerry_value_t has_prop_js = jerry_has_property (parsed_json, key);
bool has_prop = jerry_get_boolean_value (has_prop_js);
TEST_ASSERT (has_prop == true);
jerry_release_value (has_prop_js);
Copy link
Contributor

@robertsipka robertsipka Mar 19, 2018

Choose a reason for hiding this comment

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

jerry_value_t has_key_prop = jerry_has_property (parsed_json, key);
TEST_ASSERT (jerry_get_boolean_value (has_key_prop));
jerry_release_value (has_key_prop);

bool has_prop = jerry_get_boolean_value (has_prop_js);
TEST_ASSERT (has_prop == true);
jerry_release_value (has_prop_js);
parsed_data = jerry_get_property (parsed_json, key);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can declare parsed_data here also

char buff[jerry_get_string_length (parsed_data)];
jerry_char_t *buff_p = (jerry_char_t *) buff;
jerry_string_to_char_buffer (parsed_data, buff_p, buff_size);
buff[buff_size] = '\0';
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without it there is an error in utittests, as I told you.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about using strncmp instead of strcmp?

jerry_char_t *buff_p = (jerry_char_t *) buff;
jerry_string_to_char_buffer (parsed_data, buff_p, buff_size);
buff[buff_size] = '\0';
TEST_ASSERT (strcmp ((const char *) data_check , (const char *) buff) == false);
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think it is necessary to create a variable for the data_check content, just put the string here.

jerry_string_to_char_buffer (stringified, (jerry_char_t *) buff,
(jerry_size_t) jerry_get_string_length (stringified));
buff[jerry_get_string_length (stringified)] = '\0';
TEST_ASSERT (strcmp ((const char *) check_value , (const char *) buff) == 0);
Copy link
Contributor

@robertsipka robertsipka Mar 19, 2018

Choose a reason for hiding this comment

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

double spaces.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think it is necessary to create a variable for the check_value content, just put the string here.

@robertsipka
Copy link
Contributor

robertsipka commented Mar 19, 2018

Sorry, I've just realized that this is a WIP PR.

@ZsoltRaduska ZsoltRaduska force-pushed the extendCapi branch 6 times, most recently from 8f3af57 to e49ec89 Compare March 22, 2018 14:35
@ZsoltRaduska ZsoltRaduska changed the title [WIP] Add json parse and stringify function to jerryscript c api Add json parse and stringify function to jerryscript c api Mar 22, 2018
/**
* @}
* @}
*/

#endif /* !CONFIG_DISABLE_JSON_BUILTIN */
Copy link
Contributor

@robertsipka robertsipka Mar 22, 2018

Choose a reason for hiding this comment

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

It is outside from the comment group, please put it before the end of the group.

@ZsoltRaduska ZsoltRaduska force-pushed the extendCapi branch 4 times, most recently from 1547e92 to 0c582d4 Compare March 23, 2018 07:58
**Summary**

Parse a JSON string.
Returns a jerry_value_t which contains key-value pairs imported from the JSON string.
Copy link
Member

Choose a reason for hiding this comment

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

Comment: Returns the same result as JSON.parse ecmascript function.

const char *data_in_json = "{\"name\": \"John\", \"age\": 5}";
jerry_value_t parsed_json = jerry_json_parse (data_in_json);

// parsed_json now conatins all of data stored in data_in_json
Copy link
Member

Choose a reason for hiding this comment

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

all data

**Summary**

Stringify a jerry_value_t object.
Convert a jerry_value_t object into a json formated string.
Copy link
Member

Choose a reason for hiding this comment

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

Comment: Returns the same value as JSON.stringify() ecmascript function.

bool parse_string); /**< strings are allowed to parse */
ecma_object_t *ecma_op_create_object_object_noarg (void);
ecma_value_t
ecma_op_object_put (ecma_object_t *object_p, /**< the object */
Copy link
Member

Choose a reason for hiding this comment

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

I think these functions are exported elsewhere.

@@ -814,6 +778,7 @@ ecma_builtin_json_walk (ecma_object_t *reviver_p, /**< reviver function */
* @return ecma value
* Returned value must be freed with ecma_free_value.
*/

Copy link
Member

Choose a reason for hiding this comment

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

No need this newline.

left_square_token, /**< JSON left square bracket */
comma_token, /**< JSON comma */
colon_token /**< JSON colon */
} ecma_json_token_type_t;
Copy link
Member

Choose a reason for hiding this comment

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

These types should not be exported.

@@ -152,6 +190,26 @@ typedef struct
ecma_object_t *replacer_function_p;
} ecma_json_stringify_context_t;

ecma_value_t ecma_builtin_json_parse_value (ecma_json_token_t *token_p);
void ecma_builtin_json_parse_next_token (ecma_json_token_t *token_p, /**< token argument */
bool parse_string); /**< strings are allowed to parse */
Copy link
Member

Choose a reason for hiding this comment

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

Misaligned argument.

@ZsoltRaduska ZsoltRaduska force-pushed the extendCapi branch 2 times, most recently from fe73145 to 48403f3 Compare March 26, 2018 15:24
@ZsoltRaduska ZsoltRaduska force-pushed the extendCapi branch 8 times, most recently from b535c52 to a2c01aa Compare March 27, 2018 13:56
@ZsoltRaduska ZsoltRaduska force-pushed the extendCapi branch 3 times, most recently from b79de02 to 9b96e6b Compare March 27, 2018 15:11
Copy link
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM


- `string_p` - a JSON string
- `string_size` - size of the string

Copy link
Contributor

Choose a reason for hiding this comment

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

missing return statement

```

- `object_to_stringify` - a jerry_value_t object to stringify

Copy link
Contributor

Choose a reason for hiding this comment

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

missing return statement

*
* Note:
* The returned value must be freed with jerry_release_value
* @return jerry_value_t from json formated string
Copy link
Contributor

Choose a reason for hiding this comment

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

We should mention that the return value can be an error.

*
* Note:
* The returned value must be freed with jerry_release_value
* @return json formated jerry_value_t
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -152,6 +152,9 @@ typedef struct
ecma_object_t *replacer_function_p;
} ecma_json_stringify_context_t;

ecma_value_t ecma_builtin_json_parse_buffer (const lit_utf8_byte_t * str_start_p,
lit_utf8_size_t string_size);
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong indentation

*/
ecma_value_t
ecma_builtin_json_parse_buffer (const lit_utf8_byte_t * str_start_p, /**< String to parse */
lit_utf8_size_t string_size) /**< size of the string */
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

/**
* Function to set a string token from the given arguments, fill its fields and advances the string pointer.
*
* @return ecma value
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add more details for the return value.

/**
* Helper functions to stringify an object in JSON format representing an ecma_value.
*
* @return ecma value
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

/**
* Function to create a json formated string from an object
*
* @return ecma value
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@ZsoltRaduska ZsoltRaduska force-pushed the extendCapi branch 4 times, most recently from 125a86b to 8204979 Compare March 29, 2018 15:32
@@ -805,6 +805,35 @@ ecma_builtin_json_walk (ecma_object_t *reviver_p, /**< reviver function */
return ret_value;
} /* ecma_builtin_json_walk */

/**
* Function to set a string token from the given arguments, fill its fields and advances the string pointer.
Copy link
Contributor

Choose a reason for hiding this comment

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

fills

@@ -892,6 +906,61 @@ ecma_builtin_json_object (ecma_object_t *obj_p, ecma_json_stringify_context_t *c
static ecma_value_t
ecma_builtin_json_array (ecma_object_t *obj_p, ecma_json_stringify_context_t *context_p);

/**
* Helper functions to stringify an object in JSON format representing an ecma_value.
Copy link
Contributor

Choose a reason for hiding this comment

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

function

char buff[jerry_get_string_length (parsed_data)];
jerry_char_t *buff_p = (jerry_char_t *) buff;
jerry_string_to_char_buffer (parsed_data, buff_p, buff_size);
buff[buff_size] = '\0';
Copy link
Contributor

Choose a reason for hiding this comment

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

What about using strncmp instead of strcmp?

@ZsoltRaduska ZsoltRaduska force-pushed the extendCapi branch 3 times, most recently from c3a44e7 to 3043555 Compare April 3, 2018 22:05
JerryScript-DCO-1.0-Signed-off-by: Zsolt Raduska [email protected]
Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

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

LGTM

@LaszloLango LaszloLango merged commit 78bd11e into jerryscript-project:master Apr 5, 2018
martijnthe pushed a commit to martijnthe/jerryscript that referenced this pull request Apr 6, 2018
* Add the ability to throw an error to python debugger (jerryscript-project#2188)

This patch makes it possible to throw an error from the python debugger client using the `throw` command.

JerryScript-DCO-1.0-Signed-off-by: Daniel Balla [email protected]

* Fix typo and redundant text in the documentation. (jerryscript-project#2260)

JerryScript-DCO-1.0-Signed-off-by: Daniel Vince [email protected]

* Fix accessing the contents of a direct string (jerryscript-project#2261)

In the `ecma_string_get_chars` method
the contents of a direct string is accessed incorrectly.
It tries to extract the magic string id from the
string pointer but the direct string does not need
this step.

JerryScript-DCO-1.0-Signed-off-by: Peter Gal [email protected]

* Remove websocket message macros in debugger (jerryscript-project#2262)

JERRY_DEBUGGER_INIT_SEND_MESSAGE
JERRY_DEBUGGER_SET_SEND_MESSAGE_SIZE
JERRY_DEBUGGER_SET_SEND_MESSAGE_SIZE_FROM_TYPE

JerryScript-DCO-1.0-Signed-off-by: Jimmy Huang [email protected]

* Remove TARGET_HOST macros (jerryscript-project#2264)

Remove TARGET_HOST defines from the jerry-libc module and replace with compiler provided macros.

JerryScript-DCO-1.0-Signed-off-by: Istvan Miklos [email protected]

* Fix bug in stringToCesu8 conversion for 0x7ff (jerryscript-project#2267)

JerryScript-DCO-1.0-Signed-off-by: Geoff Gustafson [email protected]

* Add ecma_free_all_enqueued_jobs function (jerryscript-project#2265)

This function releases any remaining promise job that wasn't completed.
Also added this function to `jerry_cleanup ()`, therefore it will be automatically run.

JerryScript-DCO-1.0-Signed-off-by: Daniel Balla [email protected]

* Improve jerry_is_feature_enabled with object availability information (jerryscript-project#2250)

JerryScript-DCO-1.0-Signed-off-by: Zsolt Raduska [email protected]

* Add json parse and stringify function to jerryscript c api (jerryscript-project#2243)

JerryScript-DCO-1.0-Signed-off-by: Zsolt Raduska [email protected]

* Simplify debugger-ws.h (jerryscript-project#2266)

Remove several macros and types from it. This change simplifies
the external debugger interface.

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg [email protected]

* Add finalize_cb to jerry_context_data_manager_t (jerryscript-project#2269)

This patch adds a new finalize_cb callback to jerry_context_data_manager_t.
The callback is run as the very last thing in jerry_cleanup, after the VM has been torn down entirely.
There was already the deinit_cb, which is run while the VM is still in the process of being torn down.
The reason the deinit_cb is not always sufficient is that there may still be objects alive (because they still being referenced) that have native pointers associated with the context manager that is being deinit'ed.
As a result, the free_cb's for those objects can get called *after* the associated context manager's deinit_cb is run. This makes cleanup of manager state that is depended on by the live objects impossible to do in the deinit_cb. That type of cleanup can only be done when all values have been torn down completely.

JerryScript-DCO-1.0-Signed-off-by: Martijn The [email protected]

* Rework snapshot generation API. (jerryscript-project#2259)

Also remove eval context support. It provides no practical advantage.

JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg [email protected]

* Implement the ES2015 version of Object.getPrototypeOf and add a test file for it (jerryscript-project#2256)

JerryScript-DCO-1.0-Signed-off-by: Peter Marki [email protected]

* Move DevTools integration code into jerry-client-ts subdir

JerryScript-DCO-1.0-Signed-off-by: Geoff Gustafson [email protected]

* Fix some things to match the new directory for TS code

JerryScript-DCO-1.0-Signed-off-by: Geoff Gustafson [email protected]
jimmy-huang pushed a commit to jimmy-huang/jerryscript that referenced this pull request May 8, 2018
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.

5 participants