PyTerminal Plugin #1801
PyTerminal Plugin #1801
Conversation
|
@antocuni how is CI pushing commits when my pre-commits file passed? 🤔 |
|
Such an awesome demo of this today Andrea! Really stoked to have this moving forward. Just to put in writing the suggestion I made on the call - the way I most often use a REPL is with <script type="py" terminal></script>This would execute the code in a worker and then launch a terminal/REPL that's hooked to that same worker. (The name I don't have strong opinions on. Could be
I wonder if having an attribute above might solve the need for a custom element - if you want a 'naked' REPL (i.e. a REPL with no other user code run before it), you just throw |
Agreed, if we have a declarative way to start a terminal from Python, it should throw if that's called in the main thread. |
|
|
+2 . Loove it! Let's go! |
|
I prefer Over on discord there was mention of allowing something "terminal like" for the main thread, but this may only be something as simple as a |
just to clarify, is the |
|
Folks, I've just found a reltively elegant way to avoid making the terminal interactive on main but fully working as expected from the worker. I did some other cleanup and I think as first iteration this plugin might be good enough. Please feel free to review current state and if you grab this branch feel free to test the If this is OK, I might as well write an integration test before merging the PR, thank you. |
|
Really like it... super clean and a great starting point |
|
Just tried this out on workers and main, it's great! |
| # avoid bootstrapping interact() if no terminal exists | ||
| if _pyterminal.PY_TERMINAL: | ||
| import code as _code | ||
| _code.interact() |
There was a problem hiding this comment.
It's a shame not to support top level await...
There was a problem hiding this comment.
@hoodmane not intentional ... and I am not sure what you are referring to. Mind to expand a bit? We do support top level await if the tag has an async attribute though, but I am trying to figure out what would be the benefit here ... or maybe you are referring to some missing field when interact(...) is called that would allow top level await within the repl?
Thanks in advance for any sort of clarification, please remember I know nothing about Python 😅
There was a problem hiding this comment.
OK, this has been discussed during yesterday community call, I am waiting for @hoodmane link/hint on how to workaround and enable the top level await if possible, thanks!
|
Finally managed to have a look at this. The discussion is big and it spanned several media (e.g., a discord thread and the PyScript FUN call, for which I saw the recording) so let me try to summarize some of my thoughts. Note that not all of these ideas necessarily apply to the upcoming release: we might decide to implement them later, but it's still good to have an idea of what is the final goal.
In my original PR (#1771) I played with two ways of intercepting stdout/stderr:
They two two very different things: approach (2) only intercept python things, and it has problems because the object that we are using is not a "full file" and it misses methods which other programs could expect (e.g., AFAIK, The reason why I couldn't use it in the worker is that plugins cannot inject JS code in the worker, and that's where I got blocked in my PR. |
|
quick answers:
|
it seems a very dense one and I think I'll need some time to fully understand :). |
it's some sort of slippery slope + the evident demonstration the architecture behind is good for some, but not for everyone ... defeating a bit the conversation around plugins and limiting the community support and creativity. To this I rather say no ... but if having a repl is really a fundamental thing to have for the project, of course we can hack internally as much as we like, but that still doesn't cover the fact "hooks are good, but not always" which I'd rather, personally, solve, specially in the RC phase of the project. edit I am not sure I've read your comment right neither ... are you suggesting we should fork XTerm? do we have even capacity to do that? |
no, I'm not saying that. I'm saying that if we can have a "good" py-terminal solution with the current hooks, that's awesome. But in case it is not possible to tweak the hooks for technical reasons, I'd rather have a "good" py-terminal-which-is-not-a-plugin than a "less good" py-terminal-which-is-a-plugin. |
we kinda agree there, my different PoV is that I want plugins to be as good as they need to be ... if we could move this forward maybe we'll have the cake and we can eat it all too pyscript/polyscript#61 |
Just an example. Without a |
|
OK... once we have this merged, even as a deliberate "first draft", I think we're good to go for two things to happen:
|
|
Just spotted another bug, unless we're expecting this not to work. Adding async to the script tag stops the terminal from working: |
|
@JoshuaLowe1002 I believe we're waiting on @hoodmane to advise @WebReflection on top level await / But this could be a post-release refinement if we don't allow or ignore |
|
Aha! It would help if i read the review... thanks :) |
for more information, see https://pre-commit.ci
|
To help make it easy for the app writer to control the location and style of the terminals (maybe I want 5 terminals in a row each with a different background color etc), could the terminal attribute be either:- <script type="py" terminal> which creates an element for me... or... <script type="py" terminal="the-id-of-the-element-i-want-the-terminal-in"> Also, maybe a "half-way house" towards styling, another attribute terminal-class="cool-terminal"? |
|
@mchilvers currently the terminal takes over the whole interpreter input/output, so multiple terminals are not supported, like it was before ... multiple terminals will be a follow up, here it's more about feature parity (or something close to it) with PyScript classic. As nobody wants 5 different pyodide bootstraps neither, I agree with you it's something to provide in the near future, just "not today" imho, as that's not super easy, if even possible, right now without having 5 pyodides so 10 to 20 seconds before you can work on that page. |
|
@mchilvers btw, this is also why there is the |
<script type="py" worker terminal="output">
# code
# questions:
# * print bootstraps terminal? NOTE: default on PSDC to have terminal attribute
# * which backend for the terminal? NOT A BLOCKER + FOLLOW UP + XTerm.js so far so good
#
# * slightly unrelated (affects also error plugin):
# * take over the console.error? (maybe nope?)
# runtime error: here to explain current issue
def shenanigans():
print("fails", whatever())
js.setTimeout(shenanigans)
</script>
<div id="output"></div> |
PyTerminal Specs
edit now that I think about it, the |
|
Sorry to have missed the community call - I think these specs are really quite good! I would maybe suggest some changes to the way the |
|
thanks @JeffersGlass, you've been missed! Anyway, I have adjusted specs around |
|
FYI I've converted this into draft as almost nothing in this MR fulfills the specs we agreed on for the next PyScript Terminal |
|
I think in general I understand your edit/amend, and agree!
This is waiting on Hood/implementation details there, yeah? Or is there something else that would block top level await working in a worker? |
Yes that's right. Sorry for the delay, I'm going to write something for this right now. |
|
I don't want to sidetrack the conversation and prevent a release, and i may have misunderstood what's happening, however: After some experimenting, I think the error that I was unable to display in the terminal is a bug with the terminal implementation, rather than us not being able to catch it via Pyodide...
Nothing shows in the terminal, because it seems that the code in However this error does show in the main thread terminal, because from what I can see, the terminal is initiated in a different way. This obviously doesn't solve the runtime error issue, however, I think it covers showing errors that beginners will cause. |
|
My suggestion is to use My guess is the following code will work, but I haven't tested it yet. I'll try to test it soon, but let me know if you try it and run into trouble. Detailsimport sys
from pyodide import console
class Console(console.PyodideConsole):
def write(self, txt):
if self.stdout_callback:
self.stdout_callback(txt)
else:
print(txt)
def raw_input(self, prompt=""):
"""Write a prompt and read a line.
The returned line does not include the trailing newline.
When the user enters the EOF key sequence, EOFError is raised.
The base implementation uses the built-in function
input(); a subclass may replace this with a different
implementation.
"""
return input(prompt)
async def interact(self, banner=None, exitmsg=None):
"""Closely emulate the interactive Python console.
The optional banner argument specifies the banner to print
before the first interaction; by default it prints a banner
similar to the one printed by the real Python interpreter,
followed by the current class name in parentheses (so as not
to confuse this with the real interpreter -- since it's so
close!).
The optional exitmsg argument specifies the exit message
printed when exiting. Pass the empty string to suppress
printing an exit message. If exitmsg is not given or None,
a default message is printed.
"""
try:
sys.ps1
except AttributeError:
sys.ps1 = ">>> "
try:
sys.ps2
except AttributeError:
sys.ps2 = "... "
cprt = 'Type "help", "copyright", "credits" or "license" for more information.'
if banner is None:
banner = "Python %s on %s\n%s\n(%s)\n" % (
sys.version,
sys.platform,
cprt,
self.__class__.__name__,
)
self.write("%s\n" % str(banner))
more = 0
while 1:
try:
if more:
prompt = sys.ps2
else:
prompt = sys.ps1
try:
line = self.raw_input(prompt)
except EOFError:
self.write("\n")
break
else:
fut = self.push(line)
more = fut.syntax_check == "incomplete"
# raises an error if there was a syntax error or error
res = await fut
if res:
self.write(
console.repr_shorten(
res, separator="\n<long output truncated>\n"
)
)
except KeyboardInterrupt:
self.write("\nKeyboardInterrupt\n")
self.buffer = []
more = 0
if exitmsg is None:
self.write("now exiting %s...\n" % self.__class__.__name__)
elif exitmsg != "":
self.write("%s\n" % exitmsg) |
|
I'm sorry I missed the meeting this morning, I had to go to the DMV... |
|
I agree with all the specs, with two minor points:
what happens if we have two tags running in the main thread: <script type="py">print("hello")</script>
<script type="py" terminal>print("world")</script>The stdout redirection is per-interpreter and these two tags share the same interpreter. I expect that they would print
I firmly believe that we should never user console for warnings and errors. |
Hmmm... I'd put it more like, "*by default warnings and errors should go to the DOM (but this can be turned off)". I can think of some cases where app authors don't want this sort of behaviour. E.g. a non-beta "final" version of their code may not display such things, but log them silently in a way that allows the app author to see what their non-technical user is doing. |
|
@JoshuaLowe1002 I will check that but the error plugin doesn't work for workers ... this MR should stop having anyone trying anything as it is not what we'll ship. I am saying this so that nobody would waste time on this MR, thanks for your understanding. |
|
This can be closed after this MR that just landed: #1816 |
Description
This MR takes over previous attempt #1771 simplifying a couple of things, improving foreign dependencies import performance and using a standard custom-element.
This MR makes the terminal interactive only if the
<py-terminal>tag is present on the page and only if the running script uses aworkerattribute.Please Note the terminal currently doesn't work in MicroPython
worker(interactive mode) as thecodemodule is missing and I have errors in redefiningsys.stdoutand friends.Changes
py-terminal.jsplugin that bootstraps correctlypy-terminalfrom either main or worker, actually interactive only within the latter casepyterminal.pyfile to be sure it does what it does only once and only if in workerimport pyterminalfrom a worker, appending the element if not present on the page, and let the rest of the plugin work as expectedChecklist
docs/changelog.md