After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 722285 - logError: format syntax error differently
logError: format syntax error differently
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2014-01-15 18:46 UTC by Giovanni Campagna
Modified: 2014-01-15 21:40 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
logError: format syntax error differently (6.13 KB, patch)
2014-01-15 18:46 UTC, Giovanni Campagna 		committed Details | Review

Description Giovanni Campagna 2014-01-15 18:46:00 UTC 
The stack of a syntax error shows the importing module, not the
imported one, so the information of which line has the error is
lost.
Also, syntax errors are definitely programmer errors, and should
be criticals rather than warnings.
Comment 1 Giovanni Campagna 2014-01-15 18:46:03 UTC 
Created attachment 266376 [details] [review]
logError: format syntax error differently
Comment 2 Jasper St. Pierre (not reading bugmail) 2014-01-15 19:23:08 UTC 
Review of attachment 266376 [details] [review]:

::: gjs/context.cpp
@@ +778,3 @@
+              "Script evaluation succeeded");
+
+    if (gjs_log_exception(js_context->context)) {

We'll already return FALSE from gjs_eval_with_scope for this -- we have a JS_IsExceptionPending.

::: gjs/jsapi-util.cpp
@@ +505,3 @@
         utf8_message = NULL;
 
+    if (!is_syntax) {

A comment here about why syntax errors are handled specially would be nice. Would also be nice if this was if (is_syntax) {} else {}
Comment 3 Giovanni Campagna 2014-01-15 20:38:45 UTC 
(In reply to comment #2)
> Review of attachment 266376 [details] [review]:
> 
> ::: gjs/context.cpp
> @@ +778,3 @@
> +              "Script evaluation succeeded");
> +
> +    if (gjs_log_exception(js_context->context)) {
> 
> We'll already return FALSE from gjs_eval_with_scope for this -- we have a
> JS_IsExceptionPending.

Speaking of which, I believe you introduced a regression with that, I keep getting SEGV if a script throws while loading.
Comment 4 Giovanni Campagna 2014-01-15 21:40:14 UTC 
Pushed after addressing the comments.

Attachment 266376 [details] pushed as af1e119 - logError: format syntax error differently