WIP: py-terminal plugin #1771
WIP: py-terminal plugin #1771
Conversation
| import { hooks } from "../core.js"; | ||
|
|
||
| // XXX TODO: | ||
| // 1. these imports should be lazy? |
There was a problem hiding this comment.
const { Terminal } = await import(/* webpackIgnore: true */ "https://cdn.jsdelivr.net/npm/xterm@5.3.0/+esm");
const { Readline } = await import(/* webpackIgnore: true */ "https://cdn.jsdelivr.net/npm/xterm-readline@1.1.1/+esm");There was a problem hiding this comment.
top level await should work out of the box, this file is lazily imported anyway ... we could also decide to install these as peer dependencies or something and let the bundler bring in everything we need?
There was a problem hiding this comment.
we could also decide to install these as peer dependencies or something and let the bundler bring in everything we need?
I don't have any strong opinion about that is the best way to go, happy to do whatever you think it's best
|
|
||
| // XXX TODO: | ||
| // 1. these imports should be lazy? | ||
| // 2. would be nice to automatically add xterm.css on demand |
There was a problem hiding this comment.
we can grab its content and append it on document.head ... as long as the document is the right one (see workers).
There was a problem hiding this comment.
I was thinking of dynamically adding a <link> element.
we can grab its content and append it on document.head
is there any advantage in doing this?
There was a problem hiding this comment.
the concept is the same, we add dynamically a tag or its content to the document.head and my "we can grab its content" was poorly worded: anyway we want to do so works, we still need to grab its content and append to the document.head either as link or style ... that's not what I was after, rather the fact in worker cases only we need that exsting or proxied document and add the style content in it.
There was a problem hiding this comment.
on the other hand, the link coudl be likely be added out of the box from main, no need to branch logic for workers cases only.
| console.log("hello onInterpreterReady"); | ||
| const t = makePyTerminal(); | ||
| if (!t) { | ||
| console.log("<py-terminal> not found, nothing to do"); |
There was a problem hiding this comment.
console warn maybe? if we enable this by default the bootstrap time will definitively increase (and in such case it should be a console.debug not a log) while if this is an explicit opt-in I would expect actually a console.warn instead.
There was a problem hiding this comment.
ah yes, these are just for development/debugging, they should probably just go away
| @@ -0,0 +1,110 @@ | |||
| // PyScript py-terminal plugin | |||
There was a problem hiding this comment.
shouldn't this be called py-terminal.js instead? 🤔
86bb287 to
89c2124
Compare
|
this is still very very WIP and needs to be polished, but it's working enough to allow pyscript/pyscript.core/test/pyterminal.html Lines 17 to 32 in eaf4da7
|
|
AFAIK this is a blocker for the next release, see #1786 I wonder if the WIP state should hint it's too early to keep reviewing this but if that's not the case please let me know so I can go through the code (I've seen a few pushes since last time I've checked) and hopefully help moving forward. As side note, if there is any particular blocker please either file an issue per each blocker (or a discussion if you feel like that's needed) so that we can track side-issues/blockers around this plugin, thank you! |
for more information, see https://pre-commit.ci
|
I think this MR took over this WIP so, for clean-up duty sake, I'll just remove it from the pending MR. I hope that's OK. |
WIP