[gjs: 1/4] print: Connect up prettyPrint() to Console.interact() and log()

[Philip Chimento <pchimento src gnome org>]

commit 410748846ee9b34db67d84abf95b6c7b7cac75a6
Author: Philip Chimento <philip chimento gmail com>
Date:   Wed Mar 17 22:17:46 2021 -0700

    print: Connect up prettyPrint() to Console.interact() and log()
    
    It makes sense for the prettyPrint() function to be written in JS, but it
    does need to be called internally in Console.interact() which is written
    in C++. Therefore, create a slot on the global object in which to store
    the prettyPrint() function, which can be retrieved when needing to call
    the function from internal code.
    
    Console.interact() no longer needs to use gjs_value_debug_string(), so
    that code and its dependencies can be removed.
    
    Also pass the arguments of log() and logError() through prettyPrint().
    
    This enables the work from #107.

 gjs/global.h                         |   2 +
 gjs/jsapi-util.cpp                   | 115 -----------------------------------
 gjs/jsapi-util.h                     |   3 -
 modules/console.cpp                  |  40 +++++++++++-
 modules/print.cpp                    |  28 +++++++++
 modules/script/_bootstrap/default.js |  19 +++++-
 test/gjs-tests.cpp                   |  46 --------------
 7 files changed, 86 insertions(+), 167 deletions(-)
---
diff --git a/gjs/global.h b/gjs/global.h
index 569a8ce16..bcfef51e9 100644
--- a/gjs/global.h
+++ b/gjs/global.h
@@ -42,6 +42,8 @@ enum class GjsGlobalSlot : uint32_t {
     // Stores the module registry (a Map object)
     MODULE_REGISTRY,
     NATIVE_REGISTRY,
+    // prettyPrint() function defined in JS but used internally in C++
+    PRETTY_PRINT_FUNC,
     PROTOTYPE_gtype,
     PROTOTYPE_importer,
     PROTOTYPE_function,
diff --git a/gjs/jsapi-util.cpp b/gjs/jsapi-util.cpp
index 4168f3acb..4ac9782b3 100644
--- a/gjs/jsapi-util.cpp
+++ b/gjs/jsapi-util.cpp
@@ -243,121 +243,6 @@ JSObject* gjs_define_string_array(JSContext* context,
     return array;
 }
 
-/**
- * gjs_string_readable:
- *
- * Return a string that can be read back by gjs-console; for
- * JS strings that contain valid Unicode, we return a UTF-8 formatted
- * string.  Otherwise, we return one where non-ASCII-printable bytes
- * are \x escaped.
- *
- */
-[[nodiscard]] static std::string gjs_string_readable(JSContext* context,
-                                                     JS::HandleString string) {
-    std::string buf(1, '"');
-
-    JS::UniqueChars chars(JS_EncodeStringToUTF8(context, string));
-    if (!chars) {
-        /* I'm not sure this code will actually ever be reached except in the
-         * case of OOM, since JS_EncodeStringToUTF8() seems to happily output
-         * non-valid UTF-8 bytes. However, let's leave this in, since
-         * SpiderMonkey may decide to do validation in the future. */
-
-        /* Find out size of buffer to allocate, not counting 0-terminator */
-        size_t len = JS_PutEscapedString(context, NULL, 0, string, '"');
-        char *escaped = g_new(char, len + 1);
-
-        JS_PutEscapedString(context, escaped, len, string, '"');
-        buf += escaped;
-        g_free(escaped);
-    } else {
-        buf += chars.get();
-    }
-
-    return buf + '"';
-}
-
-[[nodiscard]] static std::string _gjs_g_utf8_make_valid(const char* name) {
-    const char *remainder, *invalid;
-    int remaining_bytes, valid_bytes;
-
-    g_return_val_if_fail (name != NULL, "");
-
-    remainder = name;
-    remaining_bytes = strlen (name);
-
-    if (remaining_bytes == 0)
-        return std::string(name);
-
-    std::string buf;
-    buf.reserve(remaining_bytes);
-    while (remaining_bytes != 0) {
-        if (g_utf8_validate (remainder, remaining_bytes, &invalid))
-            break;
-        valid_bytes = invalid - remainder;
-
-        buf.append(remainder, valid_bytes);
-        /* append U+FFFD REPLACEMENT CHARACTER */
-        buf += "\357\277\275";
-
-        remaining_bytes -= valid_bytes + 1;
-        remainder = invalid + 1;
-    }
-
-    buf += remainder;
-
-    g_assert(g_utf8_validate(buf.c_str(), -1, nullptr));
-
-    return buf;
-}
-
-/**
- * gjs_value_debug_string:
- * @context:
- * @value: Any JavaScript value
- *
- * Returns: A UTF-8 encoded string describing @value
- */
-std::string gjs_value_debug_string(JSContext* context, JS::HandleValue value) {
-    /* Special case debug strings for strings */
-    if (value.isString()) {
-        JS::RootedString str(context, value.toString());
-        return gjs_string_readable(context, str);
-    }
-
-    JS::RootedString str(context, JS::ToString(context, value));
-
-    if (!str) {
-        JS_ClearPendingException(context);
-        str = JS_ValueToSource(context, value);
-    }
-
-    if (!str) {
-        if (value.isObject()) {
-            /* Specifically the Call object (see jsfun.c in spidermonkey)
-             * does not have a toString; there may be others also.
-             */
-            const JSClass* klass = JS::GetClass(&value.toObject());
-            if (klass != NULL) {
-                str = JS_NewStringCopyZ(context, klass->name);
-                JS_ClearPendingException(context);
-                if (!str)
-                    return "[out of memory copying class name]";
-            } else {
-                gjs_log_exception(context);
-                return "[unknown object]";
-            }
-        } else {
-            return "[unknown non-object]";
-        }
-    }
-
-    g_assert(str);
-
-    JS::UniqueChars bytes = JS_EncodeStringToUTF8(context, str);
-    return _gjs_g_utf8_make_valid(bytes.get());
-}
-
 // Helper function: perform ToString on an exception (which may not even be an
 // object), except if it is an InternalError, which would throw in ToString.
 GJS_JSAPI_RETURN_CONVENTION
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index 616038b23..0724181e3 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -450,9 +450,6 @@ bool gjs_log_exception_uncaught(JSContext* cx);
 void gjs_log_exception_full(JSContext* cx, JS::HandleValue exc,
                             JS::HandleString message, GLogLevelFlags level);
 
-[[nodiscard]] std::string gjs_value_debug_string(JSContext* cx,
-                                                 JS::HandleValue value);
-
 void gjs_warning_reporter(JSContext*, JSErrorReport* report);
 
 GJS_JSAPI_RETURN_CONVENTION
diff --git a/modules/console.cpp b/modules/console.cpp
index e54554b6d..f92d63d07 100644
--- a/modules/console.cpp
+++ b/modules/console.cpp
@@ -28,6 +28,7 @@
 #include <glib/gprintf.h>  // for g_fprintf
 
 #include <js/CallArgs.h>
+#include <js/CharacterEncoding.h>  // for JS_EncodeStringToUTF8
 #include <js/CompilationAndEvaluation.h>
 #include <js/CompileOptions.h>
 #include <js/ErrorReport.h>
@@ -35,11 +36,15 @@
 #include <js/RootingAPI.h>
 #include <js/SourceText.h>
 #include <js/TypeDecls.h>
+#include <js/Utility.h>  // for UniqueChars
+#include <js/Value.h>
+#include <js/ValueArray.h>
 #include <js/Warnings.h>
 #include <jsapi.h>  // for JS_IsExceptionPending, Exce...
 
 #include "gjs/atoms.h"
 #include "gjs/context-private.h"
+#include "gjs/global.h"
 #include "gjs/jsapi-util.h"
 #include "modules/console.h"
 
@@ -146,11 +151,26 @@ sigjmp_buf AutoCatchCtrlC::jump_buffer;
     return true;
 }
 
+std::string print_string_value(JSContext* cx, JS::HandleValue v_string) {
+    if (!v_string.isString())
+        return "[unexpected result from printing value]";
+
+    JS::RootedString printed_string(cx, v_string.toString());
+    JS::AutoSaveExceptionState exc_state(cx);
+    JS::UniqueChars chars(JS_EncodeStringToUTF8(cx, printed_string));
+    exc_state.restore();
+    if (!chars)
+        return "[error printing value]";
+
+    return chars.get();
+}
+
 /* Return value of false indicates an uncatchable exception, rather than any
  * exception. (This is because the exception should be auto-printed around the
  * invocation of this function.)
  */
 [[nodiscard]] static bool gjs_console_eval_and_print(JSContext* cx,
+                                                     JS::HandleObject global,
                                                      const std::string& bytes,
                                                      int lineno) {
     JS::SourceText<mozilla::Utf8Unit> source;
@@ -173,7 +193,23 @@ sigjmp_buf AutoCatchCtrlC::jump_buffer;
     if (result.isUndefined())
         return true;
 
-    g_fprintf(stdout, "%s\n", gjs_value_debug_string(cx, result).c_str());
+    JS::AutoSaveExceptionState exc_state(cx);
+    JS::RootedValue v_printed_string(cx);
+    JS::RootedValue v_pretty_print(
+        cx, gjs_get_global_slot(global, GjsGlobalSlot::PRETTY_PRINT_FUNC));
+    bool ok = JS::Call(cx, global, v_pretty_print, JS::HandleValueArray(result),
+                       &v_printed_string);
+    if (!ok)
+        gjs_log_exception(cx);
+    exc_state.restore();
+
+    if (ok) {
+        g_fprintf(stdout, "%s\n",
+                  print_string_value(cx, v_printed_string).c_str());
+    } else {
+        g_fprintf(stdout, "[error printing value]\n");
+    }
+
     return true;
 }
 
@@ -251,7 +287,7 @@ gjs_console_interact(JSContext *context,
         bool ok;
         {
             AutoReportException are(context);
-            ok = gjs_console_eval_and_print(context, buffer, startline);
+            ok = gjs_console_eval_and_print(context, global, buffer, startline);
         }
         exit_warning = false;
 
diff --git a/modules/print.cpp b/modules/print.cpp
index ad39058b0..2b07a27b0 100644
--- a/modules/print.cpp
+++ b/modules/print.cpp
@@ -17,8 +17,10 @@
 #include <js/RootingAPI.h>
 #include <js/TypeDecls.h>
 #include <js/Utility.h>  // for UniqueChars
+#include <js/Value.h>
 #include <jsapi.h>
 
+#include "gjs/global.h"
 #include "gjs/jsapi-util.h"
 #include "modules/print.h"
 
@@ -135,12 +137,38 @@ static bool gjs_printerr(JSContext* context, unsigned argc, JS::Value* vp) {
     return true;
 }
 
+// The pretty-print functionality is best written in JS, but needs to be used
+// from C++ code. This stores the prettyPrint() function in a slot on the global
+// object so that it can be used internally by the Console module.
+// This function is not available to user code.
+GJS_JSAPI_RETURN_CONVENTION
+static bool set_pretty_print_function(JSContext*, unsigned argc,
+                                      JS::Value* vp) {
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+
+    // can only be called internally, so OK to assert correct arguments
+    g_assert(args.length() == 2 && "setPrettyPrintFunction takes 2 arguments");
+
+    JS::Value v_global = args[0];
+    JS::Value v_func = args[1];
+
+    g_assert(v_global.isObject() && "first argument must be an object");
+    g_assert(v_func.isObject() && "second argument must be an object");
+
+    gjs_set_global_slot(&v_global.toObject(), GjsGlobalSlot::PRETTY_PRINT_FUNC,
+                        v_func);
+
+    args.rval().setUndefined();
+    return true;
+}
+
 // clang-format off
 static constexpr JSFunctionSpec funcs[] = {
     JS_FN("log", gjs_log, 1, GJS_MODULE_PROP_FLAGS),
     JS_FN("logError", gjs_log_error, 2, GJS_MODULE_PROP_FLAGS),
     JS_FN("print", gjs_print, 0, GJS_MODULE_PROP_FLAGS),
     JS_FN("printerr", gjs_printerr, 0, GJS_MODULE_PROP_FLAGS),
+    JS_FN("setPrettyPrintFunction", set_pretty_print_function, 1, GJS_MODULE_PROP_FLAGS),
     JS_FS_END};
 // clang-format on
 
diff --git a/modules/script/_bootstrap/default.js b/modules/script/_bootstrap/default.js
index 952d7fe35..7d77a2cd5 100644
--- a/modules/script/_bootstrap/default.js
+++ b/modules/script/_bootstrap/default.js
@@ -5,7 +5,23 @@
 (function (exports) {
     'use strict';
 
-    const {print, printerr, log, logError} = imports._print;
+    const {
+        print,
+        printerr,
+        log: nativeLog,
+        logError: nativeLogError,
+        setPrettyPrintFunction,
+    } = imports._print;
+
+    function log(...args) {
+        return nativeLog(args.map(prettyPrint).join(' '));
+    }
+
+    function logError(e, ...args) {
+        if (args.length === 0)
+            return nativeLogError(e);
+        return nativeLogError(e, args.map(prettyPrint).join(' '));
+    }
 
     Object.defineProperties(exports, {
         ARGV: {
@@ -41,4 +57,5 @@
             value: logError,
         },
     });
+    setPrettyPrintFunction(exports, prettyPrint);
 })(globalThis);
diff --git a/test/gjs-tests.cpp b/test/gjs-tests.cpp
index 122e36390..d84c7c985 100644
--- a/test/gjs-tests.cpp
+++ b/test/gjs-tests.cpp
@@ -17,7 +17,6 @@
 #include <glib.h>
 #include <glib/gstdio.h>  // for g_unlink
 
-#include <js/Array.h>
 #include <js/BigInt.h>
 #include <js/CharacterEncoding.h>
 #include <js/Exception.h>
@@ -27,7 +26,6 @@
 #include <js/TypeDecls.h>
 #include <js/Utility.h>  // for UniqueChars
 #include <js/Value.h>
-#include <js/ValueArray.h>
 #include <jsapi.h>
 #include <jspubtd.h>  // for JSProto_Number
 #include <mozilla/Span.h>  // for MakeStringSpan
@@ -687,44 +685,6 @@ static void test_jsapi_util_string_to_ucs4(GjsUnitTestFixture* fx,
     g_free(chars);
 }
 
-static void test_jsapi_util_debug_string_valid_utf8(GjsUnitTestFixture* fx,
-                                                    const void*) {
-    JS::RootedValue v_string(fx->cx);
-    g_assert_true(gjs_string_from_utf8(fx->cx, VALID_UTF8_STRING, &v_string));
-
-    std::string debug_output = gjs_value_debug_string(fx->cx, v_string);
-
-    g_assert_cmpstr("\"" VALID_UTF8_STRING "\"", ==, debug_output.c_str());
-}
-
-static void test_jsapi_util_debug_string_invalid_utf8(GjsUnitTestFixture* fx,
-                                                      const void*) {
-    g_test_skip("SpiderMonkey doesn't validate UTF-8 after encoding it");
-
-    JS::RootedValue v_string(fx->cx);
-    const char16_t invalid_unicode[] = { 0xffff, 0xffff };
-    v_string.setString(JS_NewUCStringCopyN(fx->cx, invalid_unicode, 2));
-
-    std::string debug_output = gjs_value_debug_string(fx->cx, v_string);
-    // g_assert_cmpstr("\"\\xff\\xff\\xff\\xff\"", ==, debug_output.c_str());
-}
-
-static void test_jsapi_util_debug_string_object_with_complicated_to_string(
-    GjsUnitTestFixture* fx, const void*) {
-    const char16_t desserts[] = {
-        0xd83c, 0xdf6a,  /* cookie */
-        0xd83c, 0xdf69,  /* doughnut */
-    };
-    JS::RootedValueArray<2> contents(fx->cx);
-    contents[0].setString(JS_NewUCStringCopyN(fx->cx, desserts, 2));
-    contents[1].setString(JS_NewUCStringCopyN(fx->cx, desserts + 2, 2));
-    JS::RootedObject array(fx->cx, JS::NewArrayObject(fx->cx, contents));
-    JS::RootedValue v_array(fx->cx, JS::ObjectValue(*array));
-    std::string debug_output = gjs_value_debug_string(fx->cx, v_array);
-
-    g_assert_cmpstr(u8"🍪,🍩", ==, debug_output.c_str());
-}
-
 static void test_gjs_debug_id_string_no_quotes(GjsUnitTestFixture* fx,
                                                const void*) {
     jsid id = gjs_intern_string_to_id(fx->cx, "prop_key");
@@ -1164,12 +1124,6 @@ main(int    argc,
                         test_jsapi_util_string_char16_data);
     ADD_JSAPI_UTIL_TEST("string/to_ucs4",
                         test_jsapi_util_string_to_ucs4);
-    ADD_JSAPI_UTIL_TEST("debug_string/valid-utf8",
-                        test_jsapi_util_debug_string_valid_utf8);
-    ADD_JSAPI_UTIL_TEST("debug_string/invalid-utf8",
-                        test_jsapi_util_debug_string_invalid_utf8);
-    ADD_JSAPI_UTIL_TEST("debug_string/object-with-complicated-to-string",
-                        test_jsapi_util_debug_string_object_with_complicated_to_string);
 
     ADD_JSAPI_UTIL_TEST("gi/args/safe-integer/max",
                         gjstest_test_safe_integer_max);