repl: improve preview length calculation

The preview had an off by one error in case colors where deactivated
and did not take fullwidth unicode characters into account when
displaying the preview.

PR-URL: nodejs#31112
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: BridgeAR

lib/internal/repl/utils.js

@@ -31,26 +31,19 @@ const {
 } = require('readline');
 
 const {
-  commonPrefix
+  commonPrefix,
+  getStringWidth,
 } = require('internal/readline/utils');
 
 const { inspect } = require('util');
 
 const debug = require('internal/util/debuglog').debuglog('repl');
 
-const inspectOptions = {
-  depth: 1,
+const previewOptions = {
   colors: false,
-  compact: true,
-  breakLength: Infinity
-};
-// Specify options that might change the output in a way that it's not a valid
-// stringified object anymore.
-const inspectedOptions = inspect(inspectOptions, {
   depth: 1,
-  colors: false,
   showHidden: false
-});
+};
 
 // If the error is that we've unexpectedly ended the input,
 // then let the user try to recover by adding more input.
@@ -254,7 +247,7 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
         }
         const { result } = preview;
         if (result.value !== undefined) {
-          callback(null, inspect(result.value, inspectOptions));
+          callback(null, inspect(result.value, previewOptions));
         // Ignore EvalErrors, SyntaxErrors and ReferenceErrors. It is not clear
         // where they came from and if they are recoverable or not. Other errors
         // may be inspected.
@@ -264,8 +257,19 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
                      result.className === 'ReferenceError')) {
           callback(null, null);
         } else if (result.objectId) {
+          // The writer options might change and have influence on the inspect
+          // output. The user might change e.g., `showProxy`, `getters` or
+          // `showHidden`. Use `inspect` instead of `JSON.stringify` to keep
+          // `Infinity` and similar intact.
+          const inspectOptions = inspect({
+            ...repl.writer.options,
+            colors: false,
+            depth: 1,
+            compact: true,
+            breakLength: Infinity
+          }, previewOptions);
           session.post('Runtime.callFunctionOn', {
-            functionDeclaration: `(v) => util.inspect(v, ${inspectedOptions})`,
+            functionDeclaration: `(v) => util.inspect(v, ${inspectOptions})`,
             objectId: result.objectId,
             arguments: [result]
           }, (error, preview) => {
@@ -337,15 +341,33 @@ function setupPreview(repl, contextSymbol, bufferSymbol, active) {
       // Limit the output to maximum 250 characters. Otherwise it becomes a)
       // difficult to read and b) non terminal REPLs would visualize the whole
       // output.
-      const maxColumns = MathMin(repl.columns, 250);
-
-      if (inspected.length > maxColumns) {
-        inspected = `${inspected.slice(0, maxColumns - 6)}...`;
+      let maxColumns = MathMin(repl.columns, 250);
+
+      // Support unicode characters of width other than one by checking the
+      // actual width.
+      // TODO(BridgeAR): Add a test case to verify full-width characters work as
+      // expected. Also test that the line break in case of deactivated colors
+      // work as expected.
+      if (inspected.length * 2 >= maxColumns &&
+          getStringWidth(inspected) > maxColumns) {
+        maxColumns -= 4 + (repl.useColors ? 0 : 3);
+        let res = '';
+        for (const char of inspected) {
+          maxColumns -= getStringWidth(char);
+          if (maxColumns < 0)
+            break;
+          res += char;
+        }
+        inspected = `${res}...`;
       }
+
+      // Line breaks are very rare and probably only occur in case of error
+      // messages with line breaks.
       const lineBreakPos = inspected.indexOf('\n');
       if (lineBreakPos !== -1) {
         inspected = `${inspected.slice(0, lineBreakPos)}`;
       }
+
       const result = repl.useColors ?
         `\u001b[90m${inspected}\u001b[39m` :
         `// ${inspected}`;

test/parallel/test-repl-history-navigation.js

@@ -85,7 +85,7 @@ const tests = [
                  '144, 169, 196, 225, 256, 289, 324, 361, 400, 441, 484, 529,' +
                  ' 576, 625, 676, 729, 784, 841, 900, 961, 1024, 1089, 1156, ' +
                  '1225, 1296, 1369, 1444, 1521, 1600, 1681, 1764, 1849, 1936,' +
-                 ' 2025, 2116, 2209, ...',
+                 ' 2025, 2116, 2209,...',
                `${prompt}{key : {key2 :[] }}`,
                prev && '\n// { key: { key2: [] } }',
                `${prompt}555 + 909`,
@@ -100,7 +100,7 @@ const tests = [
                  '144, 169, 196, 225, 256, 289, 324, 361, 400, 441, 484, 529,' +
                  ' 576, 625, 676, 729, 784, 841, 900, 961, 1024, 1089, 1156, ' +
                  '1225, 1296, 1369, 1444, 1521, 1600, 1681, 1764, 1849, 1936,' +
-                 ' 2025, 2116, 2209, ...',
+                 ' 2025, 2116, 2209,...',
                prompt].filter((e) => typeof e === 'string'),
     clean: true
   },