Theme is lost when running .open() multiple times

[LucianBuzzo]

If you call .open() a second time, the terminal will lose its theme. This is due to the theme option being set to null here https://github.com/xtermjs/xterm.js/blob/3.2.0/src/Terminal.ts#L700
You can get around this by setting the theme option again, but its not very intuitive.

Details

• Browser and browser version: whatsmybrowser.org/b/YWN4JTB
• OS version: OS X 10.13.3
• xterm.js version: 3.2.0

Steps to reproduce

1. Instantiate a new xterm instance and open it on a DOM element
2. Remove the dom element
3. Open the instance using a new DOM element

[Tyriar]

Hmm, I guess I put that there to prevent getOption('theme') working as it could be invalid. The real bug here is probably the fact that you are able to call open twice, @LucianBuzzo what's you use case for needing it twice?

[LucianBuzzo]

@Tyriar I would consider calling .open() twice a feature! Its a valuable part of how I'm using xterm.

On https://dashboard.resin.io/ we use xterm.js 2.7.x for our webterminal. On the dashboard you are able to open a webterminal session, navigate away to another page (removing the DOM element) and then come back to the webterminal page (adding a new DOM element) and resume your session where you left off. This functionality still works using xterm 3.2.0, you just need to set the theme again after calling .open().

The ability to 'unattach' the xterm instance from the DOM and then 're-attach' it at a later date is a really powerful tool. I understand that it's not a common use case though.

[Tyriar]

@LucianBuzzo well VS Code and Hyper both do this so I think we should support it. I guess we'll just support fixing everything up when open is called a second time (as opposed to adding a close method which undoes open and disables a bunch of stuff as a result). We can also add a note to the open jsdoc indicating it can be called twice. Open to PRs.

[LucianBuzzo]

@Tyriar So AFAIK the only thing that you need to do is set the theme option again. Can you expand a little bit on why the theme is set to null in the first place? Does it get modified somehow when its passed to the renderer?

[Tyriar]

@LucianBuzzo I did that because the theme option is passed onto ColorManager and then that is the source of truth. Colors can be invalidated which could lead to confusion (eg. setting a color to "redd" will use the default color as "redd" isn't valid).

This is what VS Code does atm, I haven't actually seen a problem with the theme because I only ever call open once and if I need to reattach I just pull it off the container and put it back onto another one.

https://github.com/Microsoft/vscode/blob/af21dc79255e351a45fd4ad00a92699acaaa130c/src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts#L364-L368

Perhaps this should be the recommended way as it's fairly simple?

[Tyriar]

Maybe we should also offer a close method, if open is called a second time without close being called, it can trigger close at the start of the function?

[leojh]

This looks like it was fixed on issue #2174 commit: 322ae13 but now it's back in 4.0.1: https://github.com/xtermjs/xterm.js/blob/4.0.1/src/Terminal.ts line 608

[Tyriar]

This should get fixed in #2435