Introduce a Plugin system, implement <py-terminal> as a plugin #917
Conversation
…r to some arbitrary object
|
|
||
| configure(config: AppConfig) { | ||
| // validate the terminal config and handle default values | ||
| const t = config.terminal; |
There was a problem hiding this comment.
Just a nit, but would be nice to have a more discriptive name although the code is small enough to not be too confusing 😄
There was a problem hiding this comment.
I'm following this rule here. The point is precisely that the code is short, and having a longer name would make it harder to read IMHO.
The reason why I introduce the extra var was to avoid this:
// validate the terminal config and handle default values
if (config.terminal !== undefined &&
config.terminal !== true &&
config.terminal !== false &&
config.terminal !== "auto") {
// XXX this should be a UserError as soon as we have it
const got = JSON.stringify(config.terminal);
throw new Error('Invalid value for config.terminal: the only accepted' +
`values are true, false and "auto", got "${got}".`);
}
if (config.terminal === undefined) {
config.terminal = "auto"; // default value
}That said, I'm happy to change t into a longer name if you think it leads to more readable code: what do you think it should be?
I'm serious here: an external reviewer is a better judge w.r.t readability than the original author of the code, so happy to hear your suggestions.
There was a problem hiding this comment.
FYI in that part there is no t anymore :)
pyscriptjs/src/pyodide.ts
Outdated
| @@ -54,8 +58,8 @@ export class PyodideRuntime extends Runtime { | |||
| async loadInterpreter(): Promise<void> { | |||
| logger.info('Loading pyodide'); | |||
| this.interpreter = await loadPyodide({ | |||
| stdout: console.log, | |||
| stderr: console.log, | |||
| stdout: (msg) => { this.stdio.stdout_writeline(msg); }, | |||
There was a problem hiding this comment.
Do we need to add typing here or does TS knows msg will be string?
There was a problem hiding this comment.
Uhm, good question, I don't really know.
I tried to compile with --strict and the compiler doesn't give any warning, so maybe somehow it manages to infer it. But adding an explicit type doesn't hurt, so I added it in 095f156
|
@FabioRosado I think this is ready to be merged, but I'll let you decide about the naming of |
|
I think it's okay like you said the code is small enough that it doesn't affect understanding of the code Feel free to hit merge 😄 |
This PR does two things:
Pluginclass, and pyscript will call the relevant methods/hooks at the right time. Currently, I have added only the minimal sets of hooks which were needed for this PR.<py-terminal>as a plugin.As the name suggests,
<py-terminal>implements a (very simple, so far) terminal wherestdoutandstderrgo:This is a good example which shows how plugins have a broader scope than just Custom Elements: the
PyTerminalplugins defines the<py-terminal>custom elements, but it also does more: in particular, it automatically add a<py-terminal>tag to the page if it's not present (depending on the config settings).Operating principles of
<py-terminal>I introduced a way to capture/redirect the stdout and stderr streams used by pyodide: when added to the page,
PyTerminalregister itself as aStdioprovider, and when lines are written to stdout or stderr, PyTerminal intercepts the call and shows the messages. This ultimately means that calls toprint()goes to the terminal.The terminal can work also in "autoshow" mode: if you put
<py-terminal auto>, it will start hidden and reveal itself as soon as the first line of text is written to it.Moreover, I added a
config.terminalsetting, which can have three values:true: automatically add a<py-terminal>to the page, if not already present"auto": automatically add a<py-terminal auto>to the page, if not already presentfalse: don't add anythingI chose to make
"auto"the default. This is very useful because it leads to the following behavior:display()you will never see the terminalprint(), the terminal shows up. This is much better and intuitive than the current situation in whichprint()are basically hidden and the only way to see them is to open dev tools.print(), it will just work.Further enhancements
This is intentionally the simplest possible version of a terminal, but it's already useful IMHO. The following is a random list of future enhancements, although I don't have plans to work on them soon (but maybe someone wants):
docs: I didn't write any docs, sorry 🤷♂️ . Moreover, it would be nice to show a nice message
What is it?(or similar) inautomode: the idea is that it will contain a link to the relevant docs which explains what is it, how to configure, how to turn it off, etc.dockable mode: currently
<py-terminal>is automatically added at the end of the DOM. It would be nice to have a way to dock it to the top/bottom/left/right of the page, and maybe to resize it dynamicallyminimize mode: another interesting alternative UI could be to have a "terminal icon" somewhere on the page, which displays notifications when it gets new messages: if you click on the icon, the terminal shows up
better styling: I did the minimal CSS possible, but it's likely that we can do better. For example, it would be nice to automatically get a horizontal scrollbar if the lines are too long, and/or to set a fixed vertical height and display also a vertical scrollbar
dumbvsxtermmode: this is a dumb terminal: no ANSI escape codes, no colors, etc. It would be nice to add the option to use xterm.js instead. According to the webpage, it is used a bit everywhere, including visual studio c 8000 ode. This means that for example we could runpytestinside the browser and see all the nice colors and formatting.stdinsupport: this is harder, and most likely requires to run pyscript in a web worker (because reading from stdin is a blocking syscall). But once we have it, things like e.g.pdb.set_trace()would probably work out of the box! 😱Enhancements to the Plugin system
As a follow-up of this PR, I plan to migrate to turn the loader and
register_importmapinto plugins.