Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Terminal Widget based on xtermjs #2090

Merged
merged 27 commits into from Jun 8, 2021
Merged

Terminal Widget based on xtermjs #2090

merged 27 commits into from Jun 8, 2021

Conversation

MarcSkovMadsen
Copy link
Collaborator

@MarcSkovMadsen MarcSkovMadsen commented Mar 16, 2021

#1925

Implements a Terminal Component based on xtermjs

Must Do

[x] Make it work in Jupyter Hub (See image with issue in post below).
[x] Create Reference Guide Notebook
[x] Design the api
[x] Support input
[x] Support output including links, emojis and danish characters.
[x] Fix size issues (using add on fit)
[x] Define .js and .css assets to include on bokeh model.
[x] Make it Work on Server
[x] Make it Work in notebook

Nice to Have

[] Support running subprocesses like bash, python and ipython in pty
- [] On windows (winpty?)
- [] Improve on add_periodic_callback. Should this be seperate thread or other period length?
- [x] On Linux
- [x] Handle user exit from terminal.
[] Support Addons
- [] Attach
- [x] fit
- [] Search
- [x] Web-Links
[] Make .fit calculation in .ts model 100% bullet proof and also support sizing_mode="stretch_height".
[] Determine whether to inherit from StreamIO or not. c.f. Philipp comment below.
[] Fix hacks for handling repeated input/ output
[] Enable copy-paste
[] Implement nice kill in notebook (see image in posts below).
[x] Example of using subprocess.Popen
[x] Example using streaming data from some longer running process
[x] Example of redirecting python log

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Mar 16, 2021

I just added the basic "Button Extension" described in the Custom Extension guide. It made is SO EASY to get started with a working custom extension. It took a few minutes to get a basic custom extension working.

Maybe a panel create-extension cli command doing the same could make it even more simple?

@MarcSkovMadsen
Copy link
Collaborator Author

A minimum POC is there

panel-terminal.mp4

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Mar 16, 2021

@philippjfr . If you have good ideas about the api we want feel free to provide an example. Right now it's as simple as

class Terminal(Widget):
    object = param.String(doc="The text to write in the terminal")
    out = param.String(doc="Any text written by the user in the terminal")

I will have to explore and learn because I have not working with terminals before. And not so much with redirecting logs, stdout etc. or using pty.

@philippjfr
Copy link
Member

It should probably just implement the file protocol, i.e. at minimum implement write but maybe the entire interface including read, seek, readline, readlines readable, writable, seekable, tell, close, closeable, flushed etc. Probably just start with write and then do:

import sys

term = Terminal()
sys.stdout = term
sys.stderr = term

@philippjfr philippjfr added this to the v0.12.0 milestone Mar 17, 2021
@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Mar 18, 2021

Got the basics working. The Terminal is now defined as

class Terminal(StringIO, Widget):
    # Parameters to be mapped to Bokeh model properties
    value = param.String(doc="""
        User input from the terminal""")
    object = param.String(doc="""
        System output to the terminal""")
  • I was in doubt. But I believe the Terminal is a widget with a value (~user input) and an object (output) parameter.
  • I will have to learn whether to inherit from StringIO is a good thing or not. For now it works.
  • For now I have defined the value and object as String parameters. I don't know if it should have been Bytes. I have a feeling that it is needed for some uses cases. For example term.write('\x1bc') does not clear the screen as described here Can't clear the terminal programmatically xtermjs/xterm.js#950

Do you have any input on these 3 considerations @philippjfr ?

panel_terminal.mp4

@MarcSkovMadsen MarcSkovMadsen changed the title Terminal Pane based on xtermjs Terminal Widget based on xtermjs Mar 18, 2021
@philippjfr
Copy link
Member

I was in doubt. But I believe the Terminal is a widget with a value (~user input) and an object (output) parameter.

Definitely straddling the line but if it's accepting user input, it's a widget.

I will have to learn whether to inherit from StringIO is a good thing or not. For now it works.

Don't think this is a good idea. Just satisfy the file-protocol.

For now I have defined the value and object as String parameters. I don't know if it should have been Bytes. I have a feeling that it is needed for some uses cases. For example term.write('\x1bc') does not clear the screen as described here xtermjs/xterm.js#950

Not sure about separating the value/object naming. Also what does object return? The entire history? The last output?

panel/models/terminal.ts Outdated Show resolved Hide resolved
@MarcSkovMadsen
Copy link
Collaborator Author

The object returns the last output.

The value returns the last input from the user

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Mar 20, 2021

Status: Output works, layout works (with a few hacks)
Next Step: User Input

panel_terminal3.mp4

@MarcSkovMadsen
Copy link
Collaborator Author

Got the .js and .css imports working in the notebook. And started writing the reference guide.

image

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Mar 23, 2021

When "streaming to the terminal" in the notebook I experienced

image

For now I've just reduced the range.

@MarcSkovMadsen
Copy link
Collaborator Author

When I kill the process in the notebook it looks like

image

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Mar 23, 2021

Importing the terminal extension in Jupyterlab does not work

image

Do you have any ideas on what could be the cause or solution @philippjfr ? Thanks.

@philippjfr
Copy link
Member

I would seriously discourage displaying anything in the same cell where you are loading the extension.

@philippjfr
Copy link
Member

That seems to be a bug though, it should wait on bokeh to be loaded instead of erroring. But give it a few seconds before you display something to let Bokeh load. If that works then it's a distinct bug and you shouldn't worry about it.

@MarcSkovMadsen
Copy link
Collaborator Author

I just used one cell to make it as "minimal" as possible. It's the same problem when using multiple cells @philippjfr

image

@MarcSkovMadsen
Copy link
Collaborator Author

When I don't import the terminal extension it works.

image

The problem for me is that importing the terminal works in the notebook.

@philippjfr
Copy link
Member

I'll take a look when I get a chance.

@MarcSkovMadsen
Copy link
Collaborator Author

Thanks @philippjfr . It's not urgent as I will just finalize the rest and then you can take a look as a part of the review/ rewrite you will be doing anyways. Thanks.

@philippjfr philippjfr merged commit b2ce7ac into master Jun 8, 2021
@philippjfr philippjfr deleted the xtermjs branch June 8, 2021 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants