From e8a8a107ffa15e0d53d2dae87a86e872a849fdfd Mon Sep 17 00:00:00 2001
From: casatir <romain.casati@basthon.fr>
Date: Tue, 19 Jan 2021 07:30:07 +0100
Subject: [PATCH] REPL Correct repr in InteractiveConsole (#1141)

---
 docs/project/changelog.md         |   2 +-
 src/core/jsimport.c               |   0
 src/core/jsimport.h               |   0
 src/pyodide-py/pyodide/console.py | 163 +++++++++++++++++++++++++++---
 src/templates/console.html        |  99 +++++++++---------
 src/tests/test_console.py         | 105 +++++++++++++++++--
 6 files changed, 299 insertions(+), 70 deletions(-)
 create mode 100644 src/core/jsimport.c
 create mode 100644 src/core/jsimport.h

diff --git a/docs/project/changelog.md b/docs/project/changelog.md
index 3b18362c0..5c949d091 100644
--- a/docs/project/changelog.md
+++ b/docs/project/changelog.md
@@ -50,7 +50,7 @@
 [#1033](https://github.com/iodide-project/pyodide/pull/1033)
 - JsBoundMethod is now a subclass of JsProxy, which fixes nested attribute access and various other strange bugs.
   [#1124](https://github.com/iodide-project/pyodide/pull/1124)
-- In console.html: sync behavior, full stdout/stderr support, clean namespace and bigger font [#1125](https://github.com/iodide-project/pyodide/pull/1125)
+- In console.html: sync behavior, full stdout/stderr support, clean namespace, bigger font, correct result representation, clean traceback [#1125](https://github.com/iodide-project/pyodide/pull/1125) and [#1141](https://github.com/iodide-project/pyodide/pull/1141)
 - Javascript functions imported like `from js import fetch` no longer trigger "invalid invocation" errors (issue [#461](https://github.com/iodide-project/pyodide/issues/461)) and `js.fetch("some_url")` also works now (issue [#768](https://github.com/iodide-project/pyodide/issues/461)).
 [#1126](https://github.com/iodide-project/pyodide/pull/1126)
 - Javascript bound method calls now work correctly with keyword arguments. [#1138](https://github.com/iodide-project/pyodide/pull/1138)
diff --git a/src/core/jsimport.c b/src/core/jsimport.c
new file mode 100644
index 000000000..e69de29bb
diff --git a/src/core/jsimport.h b/src/core/jsimport.h
new file mode 100644
index 000000000..e69de29bb
diff --git a/src/pyodide-py/pyodide/console.py b/src/pyodide-py/pyodide/console.py
index 3d3d67297..009dd1abe 100644
--- a/src/pyodide-py/pyodide/console.py
+++ b/src/pyodide-py/pyodide/console.py
@@ -1,8 +1,40 @@
-from typing import Optional, Callable
+from typing import Optional, Callable, Any
 import code
 import io
 import sys
 import platform
+from contextlib import contextmanager
+import builtins
+
+# this import can fail when we are outside a browser (e.g. for tests)
+try:
+    import js
+
+    _dummy_promise = js.Promise.resolve()
+    _load_packages_from_imports = js.pyodide.loadPackagesFromImports
+
+except ImportError:
+
+    class _FakePromise:
+        """A promise that mimic the JS promises.
+
+        Only `then is supported` and there is no asynchronicity.
+        execution occurs when then is call.
+
+        This is mainly to fake `load_packages_from_imports`
+        and `InteractiveConsole.run_complete` in contexts
+        where JS promises are not available (tests)."""
+
+        def __init__(self, args=None):
+            self.args = (args,) if args is not None else ()
+
+        def then(self, func, *args):
+            return _FakePromise(func(*self.args))
+
+    _dummy_promise = _FakePromise()
+
+    def _load_packages_from_imports(*args):
+        return _dummy_promise
 
 
 __all__ = ["InteractiveConsole"]
@@ -62,7 +94,9 @@ class InteractiveConsole(code.InteractiveConsole):
     """Interactive Pyodide console
 
     Base implementation for an interactive console that manages
-    stdout/stderr redirection.
+    stdout/stderr redirection. Since packages are loaded before running
+    code, `runcode` returns a JS promise. Override `sys.displayhook` to
+    catch the result of an execution.
 
     `self.stdout_callback` and `self.stderr_callback` can be overloaded.
 
@@ -91,12 +125,16 @@ class InteractiveConsole(code.InteractiveConsole):
         self._stderr = None
         self.stdout_callback = stdout_callback
         self.stderr_callback = stderr_callback
-        self._persistent_stream_redirection = persistent_stream_redirection
-        if self._persistent_stream_redirection:
+        self._streams_redirected = False
+        if persistent_stream_redirection:
             self.redirect_stdstreams()
+        self.run_complete = _dummy_promise
 
     def redirect_stdstreams(self):
         """ Toggle stdout/stderr redirections. """
+        # already redirected?
+        if self._streams_redirected:
+            return
 
         if self._stdout is None:
             # we use meta callbacks to allow self.std{out,err}_callback
@@ -127,27 +165,126 @@ class InteractiveConsole(code.InteractiveConsole):
         # actual redirection
         sys.stdout = self._stdout
         sys.stderr = self._stderr
+        self._streams_redirected = True
 
     def restore_stdstreams(self):
         """Restore stdout/stderr to the value it was before
         the creation of the object."""
-        sys.stdout = self._old_stdout
-        sys.stderr = self._old_stderr
+        if self._streams_redirected:
+            sys.stdout = self._old_stdout
+            sys.stderr = self._old_stderr
+            self._streams_redirected = False
 
-    def runcode(self, code):
-        if self._persistent_stream_redirection:
-            super().runcode(code)
+    @contextmanager
+    def stdstreams_redirections(self):
+        """Ensure std stream redirection.
+
+        This supports nesting."""
+        if self._streams_redirected:
+            yield
         else:
             self.redirect_stdstreams()
-            super().runcode(code)
+            yield
             self.restore_stdstreams()
 
+    def flush_all(self):
+        """ Force stdout/stderr flush. """
+        with self.stdstreams_redirections():
+            sys.stdout.flush()
+            sys.stderr.flush()
+
+    def runsource(self, *args, **kwargs):
+        """Force streams redirection.
+
+        Syntax errors are not thrown by runcode but here in runsource.
+        This is why we force redirection here since doing twice
+        is not an issue."""
+
+        with self.stdstreams_redirections():
+            return super().runsource(*args, **kwargs)
+
+    def runcode(self, code):
+        """Load imported packages then run code, async.
+
+        To achieve nice result representation, the interactive console
+        is fully implemented in Python. This has a major drawback:
+        packages should be loaded from here. This is why this
+        function sets the promise `self.run_complete`.
+        If you need to wait for the end of the computation,
+        you should await for it."""
+        parent_runcode = super().runcode
+        source = "\n".join(self.buffer)
+
+        def load_packages_and_run(*args):
+            def run(*args):
+                with self.stdstreams_redirections():
+                    parent_runcode(code)
+                    # in CPython's REPL, flush is performed
+                    # by input(prompt) at each new prompt ;
+                    # since we are not using input, we force
+                    # flushing here
+                    self.flush_all()
+                return _dummy_promise
+
+            return _load_packages_from_imports(source).then(run)
+
+        self.run_complete = self.run_complete.then(load_packages_and_run)
+
     def __del__(self):
-        if self._persistent_stream_redirection:
-            self.restore_stdstreams()
+        self.restore_stdstreams()
 
     def banner(self):
         """ A banner similar to the one printed by the real Python interpreter. """
         # copyied from https://github.com/python/cpython/blob/799f8489d418b7f9207d333eac38214931bd7dcc/Lib/code.py#L214
         cprt = 'Type "help", "copyright", "credits" or "license" for more information.'
-        return f"Python {platform.python_version()} {platform.python_build()} on WebAssembly VM\n{cprt}"
+        version = platform.python_version()
+        build = f"({', '.join(platform.python_build())})"
+        return f"Python {version} {build} on WebAssembly VM\n{cprt}"
+
+
+def repr_shorten(
+    value: Any, limit: int = 1000, split: Optional[int] = None, separator: str = "..."
+):
+    """Compute the string representation of `value` and shorten it
+    if necessary.
+
+    If it is longer than `limit` then return the firsts `split`
+    characters and the last `split` characters seperated by '...'.
+    Default value for `split` is `limit // 2`.
+    """
+    if split is None:
+        split = limit // 2
+    text = repr(value)
+    if len(text) > limit:
+        text = f"{text[:split]}{separator}{text[-split:]}"
+    return text
+
+
+def displayhook(value, repr: Callable[[Any], str]):
+    """A displayhook with custom `repr` function.
+
+    It is intendend to overload `sys.displayhook`. Note that monkeypatch
+    `builtins.repr` does not work in `sys.displayhook`. The pointer to
+    `repr` seems hardcoded in default `sys.displayhook` version
+    (which is written in C)."""
+    # from https://docs.python.org/3/library/sys.html#sys.displayhook
+    # If value is not None, this function prints repr(value) to
+    # sys.stdout, and saves value in builtins._. If repr(value) is not
+    # encodable to sys.stdout.encoding with sys.stdout.errors error
+    # handler (which is probably 'strict'), encode it to
+    # sys.stdout.encoding with 'backslashreplace' error handler.
+    if value is None:
+        return
+    builtins._ = None  # type: ignore
+    text = repr(value)
+    try:
+        sys.stdout.write(text)
+    except UnicodeEncodeError:
+        bytes = text.encode(sys.stdout.encoding, "backslashreplace")
+        if hasattr(sys.stdout, "buffer"):
+            sys.stdout.buffer.write(bytes)
+        else:
+            text = bytes.decode(sys.stdout.encoding, "strict")
+            sys.stdout.write(text)
+    sys.stdout.write("\n")
+    builtins._ = value  # type: ignore
diff --git a/src/templates/console.html b/src/templates/console.html
index 2abf25960..ae738744a 100644
--- a/src/templates/console.html
+++ b/src/templates/console.html
@@ -19,74 +19,73 @@
     <script>
       languagePluginLoader.then(() => {
         pyodide.runPython(`
-            from pyodide import console
+            import sys
             import js
+            from pyodide import console
 
 
-            class Console(console.InteractiveConsole):
-                def runcode(self, code):
-                    self.redirect_stdstreams()
-                    js.term.pause()
-                    js.term.runPython("\\n".join(self.buffer))
+            def displayhook(value):
+                separator = "\\n[[;orange;]<long output truncated>]\\n"
+                _repr = lambda v: console.repr_shorten(v, separator=separator)
+                return console.displayhook(value, _repr)
+
+            sys.displayhook = displayhook
+
+
+            class PyConsole(console.InteractiveConsole):
+                def __init__(self):
+                    super().__init__(persistent_stream_redirection=False)
 
                 def banner(self):
                     return f"Welcome to the Pyodide terminal emulator 🐍\\n{super().banner()}"
 
 
-            # JQuery.terminal add trailing newline that is not easy to remove
-            # with the current API. Doing our best here.
-            _c = Console(stdout_callback=lambda s: js.term.echo(s.rstrip("\\n")),
-                         stderr_callback=lambda s: js.term.error(s.rstrip("\\n")),
-                         persistent_stream_redirection=False)
-        `)
+            js.window.pyconsole = PyConsole()
+        `);
 
-        let c = pyodide.pyimport('_c');
+        let ps1 = '>>> ', ps2 = '... ';
 
-        function pushCode(line) {
-          handleResult(c.push(line));
+        async function interpreter(command) {
+            // multiline should be splitted (usefull when pasting)
+            term.pause();
+            for( const c of command.split('\n') ) {
+                const prompt = pyconsole.push(c) ? ps2 : ps1;
+                term.set_prompt(prompt);
+                await pyconsole.run_complete;
+            }
+            term.resume();
         }
 
         let term = $('body').terminal(
-          pushCode,
+          interpreter,
           {
-              greetings: c.banner(),
-            prompt: "[[;red;]>>> ]"
+            greetings: pyconsole.banner(),
+            prompt: ps1
           }
         );
 
-        window.term = term;
+        // JQuery.terminal adds a trailing newline that is not easy
+        // to remove with the current API. Doing our best here.
+        term.echo_no_newline = (function (message) {
+          // prompt can be polluted with previous echo
+          // _prompt_tail is the line part just before the prompt
+          this._prompt_tail = this._prompt_tail || '';
+          let prompt = this.get_prompt();
+          if( prompt.startsWith(this._prompt_tail) )
+            prompt = prompt.slice(this._prompt_tail.length);
+          else
+            this._prompt_tail = '';
+          // here, prompt and _prompt_tail should be correctly isolated
+          const lines = message.split('\n');
+          lines[0] = this._prompt_tail + lines[0];
+          this._prompt_tail = lines.pop();
+          this.set_prompt(this._prompt_tail + prompt);
+          if( lines.length )
+            this.echo(lines.join('\n'));
+        }).bind(term);
 
-        function handleResult(result) {
-          if (result) {
-            term.set_prompt('[[;gray;]... ]');
-          } else {
-            term.set_prompt('[[;red;]>>> ]');
-          }
-        }
-
-        term.runPython = function(code) {
-          pyodide.runPythonAsync(code).then(
-            term.handlePythonResult, term.handlePythonError
-          );
-        };
-
-        term.handlePythonResult = function(result) {
-          c.restore_stdstreams();
-          term.resume();
-          if (result === undefined) {
-            return;
-          } else if (result['_repr_html_'] !== undefined) {
-            term.echo(result['_repr_html_'], {raw: true});
-          } else {
-            term.echo(result.toString());
-          }
-        };
-
-        term.handlePythonError = function(result) {
-          c.restore_stdstreams();
-          term.resume();
-          term.error(result.toString());
-        }
+        pyconsole.stdout_callback = term.echo_no_newline;
+        pyconsole.stderr_callback = (s) => term.error(s.replace(/\n+$/, ""));
       });
     </script>
   </body>
diff --git a/src/tests/test_console.py b/src/tests/test_console.py
index 16b9f7bc1..356022900 100644
--- a/src/tests/test_console.py
+++ b/src/tests/test_console.py
@@ -1,6 +1,7 @@
 import pytest
 from pathlib import Path
 import sys
+import io
 
 sys.path.append(str(Path(__file__).parents[2] / "src" / "pyodide-py"))
 
@@ -23,15 +24,13 @@ def test_stream_redirection():
 
 
 @pytest.fixture
-def safe_stdstreams():
-    stdout = sys.stdout
-    stderr = sys.stderr
+def safe_sys_redirections():
+    redirected = sys.stdout, sys.stderr, sys.displayhook
     yield
-    sys.stdout = stdout
-    sys.stderr = stderr
+    sys.stdout, sys.stderr, sys.displayhook = redirected
 
 
-def test_interactive_console_streams(safe_stdstreams):
+def test_interactive_console_streams(safe_sys_redirections):
 
     my_stdout = ""
     my_stderr = ""
@@ -66,6 +65,12 @@ def test_interactive_console_streams(safe_stdstreams):
     shell.push("print('foobar')")
     assert my_stdout == "foo\nfoobar\n"
 
+    shell.push("print('foobar')")
+    assert my_stdout == "foo\nfoobar\nfoobar\n"
+
+    shell.push("1+1")
+    assert my_stdout == "foo\nfoobar\nfoobar\n2\n"
+
     shell.restore_stdstreams()
 
     my_stdout = ""
@@ -94,3 +99,91 @@ def test_interactive_console_streams(safe_stdstreams):
 
     print("bar")
     assert my_stdout == "foobar\n"
+
+    shell.push("print('foobar')")
+    assert my_stdout == "foobar\nfoobar\n"
+
+    shell.push("import sys")
+    shell.push("print('foobar', file=sys.stderr)")
+    assert my_stderr == "foobar\n"
+
+    shell.push("1+1")
+    assert my_stdout == "foobar\nfoobar\n2\n"
+
+
+def test_repr(safe_sys_redirections):
+    sep = "..."
+    for string in ("x" * 10 ** 5, "x" * (10 ** 5 + 1)):
+        for limit in (9, 10, 100, 101):
+            assert len(
+                console.repr_shorten(string, limit=limit, separator=sep)
+            ) == 2 * (limit // 2) + len(sep)
+
+    sys.stdout = io.StringIO()
+    console.displayhook(
+        [0] * 100, lambda v: console.repr_shorten(v, 100, separator=sep)
+    )
+    assert len(sys.stdout.getvalue()) == 100 + len(sep) + 1  # for \n
+
+
+@pytest.fixture
+def safe_selenium_sys_redirections(selenium):
+    selenium.run("_redirected = sys.stdout, sys.stderr, sys.displayhook")
+    yield
+    selenium.run("sys.stdout, sys.stderr, sys.displayhook = _redirected")
+
+
+def test_interactive_console(selenium, safe_selenium_sys_redirections):
+    def ensure_run_completed():
+        selenium.driver.execute_async_script(
+            """
+        const done = arguments[arguments.length - 1];
+        pyodide.globals.shell.run_complete.then(done);
+        """
+        )
+
+    selenium.run(
+        """
+    from pyodide.console import InteractiveConsole
+
+    result = None
+
+    def displayhook(value):
+        global result
+        result = value
+
+    shell = InteractiveConsole()
+    sys.displayhook = displayhook"""
+    )
+
+    selenium.run("shell.push('x = 5')")
+    selenium.run("shell.push('x')")
+    ensure_run_completed()
+    assert selenium.run("result") == 5
+
+    selenium.run("shell.push('x ** 2')")
+    ensure_run_completed()
+    assert selenium.run("result") == 25
+
+    selenium.run("shell.push('def f(x):')")
+    selenium.run("shell.push('    return x*x + 1')")
+    selenium.run("shell.push('')")
+    selenium.run("shell.push('[f(x) for x in range(5)]')")
+    ensure_run_completed()
+    assert selenium.run("result") == [1, 2, 5, 10, 17]
+
+    selenium.run("shell.push('def factorial(n):')")
+    selenium.run("shell.push('    if n < 2:')")
+    selenium.run("shell.push('        return 1')")
+    selenium.run("shell.push('    else:')")
+    selenium.run("shell.push('        return n * factorial(n - 1)')")
+    selenium.run("shell.push('')")
+    selenium.run("shell.push('factorial(10)')")
+    ensure_run_completed()
+    assert selenium.run("result") == 3628800
+
+    # with package load
+    selenium.run("shell.push('import pytz')")
+    selenium.run("shell.push('pytz.utc.zone')")
+    ensure_run_completed()
+    assert selenium.run("result") == "UTC"