Merge jerry_get_value_without_error and jerry_value_clear_error_flag #2350

imiklos · 2018-05-22T08:18:53Z

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

rerobika · 2018-05-22T09:53:48Z

docs/02.API-REFERENCE.md

+Get the value from an error.
+
+Extract the api value from an error. If the second argument is true
+it will release the the input error value.


Unnecessary the.

rerobika · 2018-05-22T09:53:58Z

jerry-core/api/jerry.c

+ * Get the value from an error value.
+ *
+ * Extract the api value from an error. If the second argument is true
+ * it will release the the input error value.


zherczeg

Good patch. Few more changes.

zherczeg · 2018-05-22T13:05:18Z

jerry-core/api/jerry.c

-void
-jerry_value_clear_error_flag (jerry_value_t *value_p) /**< api value */
+jerry_value_t jerry_get_value_from_error (jerry_value_t value, /**< api value */
+                                          bool release) /**< release value */


comment: release api value

zherczeg · 2018-05-22T13:09:03Z

tests/unit-core/test-api-errortype.c


    TEST_ASSERT (jerry_get_error_type (error_obj) == errors[idx]);

    jerry_release_value (error_obj);
+


Remove this newline.

zherczeg · 2018-05-22T13:10:35Z

tests/unit-core/test-api-set-and-clear-error-flag.c

@@ -27,10 +27,12 @@ main (void)
  jerry_value_set_error_flag (&obj_val);
  jerry_value_t err_val = jerry_acquire_value (obj_val);

-  jerry_value_clear_error_flag (&obj_val);
-  jerry_release_value (obj_val);
+  obj_val = jerry_get_value_from_error (obj_val, true);


I think the obj_val should be err_val in jerry_get_value_from_error (obj_val, true);,

zherczeg · 2018-05-22T13:10:49Z

tests/unit-core/test-api-set-and-clear-error-flag.c

  jerry_release_value (err_val);
+  jerry_release_value (obj_val);
+


Remove this newline.

imiklos · 2018-05-22T13:48:00Z

Thank you for the reviews, I updated the PR with these changes.

LaszloLango · 2018-05-22T13:54:25Z

docs/02.API-REFERENCE.md

+Get the value from an error.
+
+Extract the api value from an error. If the second argument is true
+it will release the input error value.


I think this won't be enough information for a new developer who is not familiar with JerryScript. Furthermore the example must show the two different use cases.

My suggestion:

Many API functions cannot be called with an error value. This function extracts the API value from an error. The second argument defines whether the input error value must be released or not. If it is set to `true`, then a [`jerry_release_value`](#jerry_release_value) function will be called for the first argument, so the error value won't be available after the call of `jerry_get_value_from_error`. The second argument should be false if both error and its represented value are needed. The first argument is returned unchanged if it is not an error.

LaszloLango · 2018-05-24T07:36:15Z

docs/02.API-REFERENCE.md

 ```

- `value_p` - pointer to an api value


value - error (api) value

release - defines whether input api value must be released

LaszloLango · 2018-05-24T07:48:45Z

docs/02.API-REFERENCE.md

+Get the value from an error.
+
+Extract the api value from an error. If the second argument is true
+it will release the input error value.


My suggestion:

Many API functions cannot be called with an error value. This function extracts the API value from an error. The second argument defines whether the input error value must be released or not. If it is set to `true`, then a [`jerry_release_value`](#jerry_release_value) function will be called for the first argument, so the error value won't be available after the call of `jerry_get_value_from_error`. The second argument should be false if both error and its represented value are needed. The first argument is returned unchanged if it is not an error.

LaszloLango · 2018-05-24T07:50:01Z

jerry-core/api/jerry.c

 */
-void
-jerry_value_clear_error_flag (jerry_value_t *value_p) /**< api value */
+jerry_value_t jerry_get_value_from_error (jerry_value_t value, /**< api value */


return value must be on separate line:

jerry_value_t jerry_get_value_from_error (jerry_value_t value, /**< api value */

LaszloLango · 2018-05-24T07:51:41Z

docs/02.API-REFERENCE.md

 ```

- `value_p` - pointer to an api value
-
 **Example**

 ```c


Please add an example for a not release error too.

LaszloLango · 2018-05-24T08:53:48Z

Related issue #2213

imiklos · 2018-05-24T09:41:43Z

I updated the PR. Thanks for the reviews.

LaszloLango

One minor deficiency, good to me after that.

LaszloLango · 2018-05-24T09:55:22Z

docs/02.API-REFERENCE.md

+
+  jerry_value_set_error_flag (&error);
+  jerry_value_t value = jerry_get_value_from_error (error, false);
+


... // both 'error' and 'value' can be used and must be released when they are no longer needed

Thanks, updated the PR.

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

LaszloLango

LGTM

zherczeg

LGTM

rerobika reviewed May 22, 2018

View reviewed changes

imiklos force-pushed the jerry_get_value_from_error branch from 1a22ad7 to baaa4ac Compare May 22, 2018 10:48

zherczeg requested changes May 22, 2018

View reviewed changes

imiklos force-pushed the jerry_get_value_from_error branch from baaa4ac to df50c20 Compare May 22, 2018 13:43

LaszloLango requested changes May 22, 2018

View reviewed changes

LaszloLango requested changes May 24, 2018

View reviewed changes

imiklos force-pushed the jerry_get_value_from_error branch from df50c20 to 359d25f Compare May 24, 2018 08:31

LaszloLango added this to the Release 2.0 milestone May 24, 2018

LaszloLango requested changes May 24, 2018

View reviewed changes

Merge jerry_get_value_without_error and jerry_value_clear_error_flag …

880ee70

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

imiklos force-pushed the jerry_get_value_from_error branch from 359d25f to 880ee70 Compare May 24, 2018 10:13

LaszloLango approved these changes May 24, 2018

View reviewed changes

zherczeg approved these changes May 25, 2018

View reviewed changes

zherczeg merged commit ac9fce1 into jerryscript-project:master May 25, 2018

zherczeg mentioned this pull request Mar 19, 2019

Nominating Robert Fancsik (@rerobika) to become a JerryScript reviewer #2799

Closed

akosthekiss mentioned this pull request Jul 3, 2019

UTF-8 variable name error #2936

Closed


		TEST_ASSERT (jerry_get_error_type (error_obj) == errors[idx]);

		jerry_release_value (error_obj);

		jerry_release_value (err_val);
		jerry_release_value (obj_val);


		jerry_value_set_error_flag (&error);
		jerry_value_t value = jerry_get_value_from_error (error, false);

Merge jerry_get_value_without_error and jerry_value_clear_error_flag #2350

Merge jerry_get_value_without_error and jerry_value_clear_error_flag #2350

Uh oh!

Conversation

imiklos commented May 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imiklos commented May 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LaszloLango commented May 24, 2018

Uh oh!

imiklos commented May 24, 2018

Uh oh!

LaszloLango left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LaszloLango left a comment

Choose a reason for hiding this comment

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!