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

Fix demo again #4193

Merged
merged 4 commits into from
Oct 9, 2022
Merged

Fix demo again #4193

merged 4 commits into from
Oct 9, 2022

Conversation

silamon
Copy link
Contributor

@silamon silamon commented Oct 9, 2022

Same as #3983.

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the breakages 😅

Comment on lines 28 to 30
this.register(toDisposable(() => {
core.onWillOpen(() => this.activate(terminal));
}));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change may not be needed? Regardless this won't work as it will only attach the onWillOpen event after the webgl renderer is disposed. The intent with that is to allow loading webgl/canvas without calling Terminal.open yet

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand it. So, I've added a try-catch around terminal.open, but it fails somewhere else then. It looks like the terminal remains broken when it needs to fallback to a different renderer such as canvas or dom.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the dom renderer back now, but the demo fails to see that the webgl didn't load, since the error is handled silently in terminal.ts now. It should be thrown again at the end of the terminal initialization.

@Tyriar Tyriar added this to the 5.1.0 milestone Oct 9, 2022
Comment on lines +518 to +521
try {
this._onWillOpen.fire(this.element);
}
catch { /* fails to load addon for some reason */ }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it 👍

@Tyriar Tyriar merged commit 96b26b2 into xtermjs:master Oct 9, 2022
Tyriar added a commit to microsoft/vscode that referenced this pull request Oct 10, 2022
- Create onSpecificOptionChange and onMultipleOptionChange helpers xtermjs/xterm.js#4195
- Improve dotted line for canvas xtermjs/xterm.js#4100
- Fix demo again xtermjs/xterm.js#4193
- Switch to Ubuntu 20.04 xtermjs/xterm.js#4192
- Fix clearTextureAtlas call as implemented on IRenderer xtermjs/xterm.js#4180
- Create theme service xtermjs/xterm.js#4188
- Ensure stale bitmap is not used when drawing new characters xtermjs/xterm.js#4189
- Lint rule for on=event emitter and rename all methods with on prefix to handle xtermjs/xterm.js#4187
- Fix a bunch of memory retention problems xtermjs/xterm.js#4185
Tyriar added a commit to microsoft/vscode that referenced this pull request Oct 10, 2022
- Create onSpecificOptionChange and onMultipleOptionChange helpers xtermjs/xterm.js#4195
- Fix demo again xtermjs/xterm.js#4193
- Switch to Ubuntu 20.04 xtermjs/xterm.js#4192
- Fix clearTextureAtlas call as implemented on IRenderer xtermjs/xterm.js#4180
- Create theme service xtermjs/xterm.js#4188
- Ensure stale bitmap is not used when drawing new characters xtermjs/xterm.js#4189
- Lint rule for on=event emitter and rename all methods with on prefix to handle xtermjs/xterm.js#4187
- Fix a bunch of memory retention problems xtermjs/xterm.js#4185
@silamon silamon deleted the fixdemoagain branch October 16, 2022 09:52
lemanschik pushed a commit to code-oss-dev/code that referenced this pull request Nov 25, 2022
- Create onSpecificOptionChange and onMultipleOptionChange helpers xtermjs/xterm.js#4195
- Fix demo again xtermjs/xterm.js#4193
- Switch to Ubuntu 20.04 xtermjs/xterm.js#4192
- Fix clearTextureAtlas call as implemented on IRenderer xtermjs/xterm.js#4180
- Create theme service xtermjs/xterm.js#4188
- Ensure stale bitmap is not used when drawing new characters xtermjs/xterm.js#4189
- Lint rule for on=event emitter and rename all methods with on prefix to handle xtermjs/xterm.js#4187
- Fix a bunch of memory retention problems xtermjs/xterm.js#4185
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants