🎯 Practical: Update xterm to v4 by Stanzilla · Pull Request #3830 · vercel/hyper · GitHub

Stanzilla · 2019-09-23T17:53:56Z

No description provided.

… renderer

Stanzilla · 2019-09-23T19:01:22Z

Can't get it to work atm, setting to WIP. The process launches but just does not show the Hyper window.

Stanzilla · 2019-09-24T18:31:49Z

TypeError: Invalid value used as weak map key
    at WeakMap.set (<anonymous>)
    at TermGroup_.bind (bundle.js:1)
    at TermGroup_.renderTerm (bundle.js:1)
    at TermGroup_.render (bundle.js:1)
    at h (bundle.js:1)
    at beginWork (bundle.js:1)
    at d (bundle.js:1)
    at f (bundle.js:1)
    at g (bundle.js:1)
    at m (bundle.js:1)
h @ bundle.js:1

on

class TermGroup_ extends react__WEBPACK_IMPORTED_MODULE_0___default.a.PureComponent {
        constructor(a, b) {
            super(a, b),
            this.bound = new WeakMap,
            this.termRefs = {},
            this.onTermRef = this.onTermRef.bind(this)
        }
        bind(a, b, c) {
            this.bound.has(a) || this.bound.set(a, {});
            const d = this.bound.get(a);
            return d[c] || (d[c] = a.bind(b, c)),
            d[c]
        }

LabhanshAgrawal · 2019-09-24T18:45:04Z

Hi, I tried to do this and I'm able to get the terminal window and the shell, but there are some random characters appearing at the top(when using webgl). You can take a look here

LabhanshAgrawal · 2019-09-24T18:48:03Z

TypeError: Invalid value used as weak map key
    at WeakMap.set (<anonymous>)
    at TermGroup_.bind (bundle.js:1)
    at TermGroup_.renderTerm (bundle.js:1)
    at TermGroup_.render (bundle.js:1)
    at h (bundle.js:1)
    at beginWork (bundle.js:1)
    at d (bundle.js:1)
    at f (bundle.js:1)
    at g (bundle.js:1)
    at m (bundle.js:1)
h @ bundle.js:1

on

class TermGroup_ extends react__WEBPACK_IMPORTED_MODULE_0___default.a.PureComponent {
        constructor(a, b) {
            super(a, b),
            this.bound = new WeakMap,
            this.termRefs = {},
            this.onTermRef = this.onTermRef.bind(this)
        }
        bind(a, b, c) {
            this.bound.has(a) || this.bound.set(a, {});
            const d = this.bound.get(a);
            return d[c] || (d[c] = a.bind(b, c)),
            d[c]
        }

@Stanzilla this is coming because of onURLAbort in term-group.js as it is undefined in props

Stanzilla · 2019-09-24T19:24:58Z

oh thank you, that's indeed better. Those random characters are the xterm atlas, I think

Stanzilla · 2019-09-24T19:49:18Z

@LabhanshAgrawal try this

LabhanshAgrawal · 2019-09-24T20:03:09Z

Facing problems with yarn install with the changed node-pty version. Also the rendererType TermOption cannot be 'webgl' in the new xterm.

Stanzilla · 2019-09-24T20:05:00Z

@LabhanshAgrawal why not? it can be if we load the plugin?

LabhanshAgrawal · 2019-09-24T20:10:00Z

That's the error I'm getting. I think since the renderer has been moved to a plugin, it would set the renderer type when loaded.
@Tyriar might be able to clarify better.

Stanzilla · 2019-09-24T20:11:25Z

Oh I see

LabhanshAgrawal · 2019-09-24T20:11:48Z

Removing the rendererType TermOption and reverting the node-pty version is working for me.

LabhanshAgrawal · 2019-09-24T20:12:30Z

The random characters at the top are also gone

Stanzilla · 2019-09-24T20:17:49Z

Yup. pushed your suggestion, looks good!

LabhanshAgrawal · 2019-09-24T20:21:03Z

Is there any specific reason for changing the node-pty version? 0.9.0-beta26 is not compiling for me (on mac)

Stanzilla · 2019-09-24T20:22:19Z

@LabhanshAgrawal yes we need it for the next Electron update. Which node version are you on?

LabhanshAgrawal · 2019-09-24T20:25:08Z

my node version is 12.6.0

Stanzilla · 2019-10-04T13:12:40Z

@GitSquared afaik all the fork did was bundling the webgl addon and enabling it. sadly @juancampa is no longer responding on the hyper project.

Stanzilla · 2019-10-04T13:20:02Z

@ivanwonder yeah, it's fine in VS code, I tried.

ivanwonder · 2019-10-04T13:58:10Z

after resize, continue to input and check if it's layout correctly. maybe it's not the problem of xterm. there are no windows on my side

Tyriar · 2019-10-04T13:59:58Z

@ivanwonder haven't seen that, something like it happened in Firefox but we fixed that bug a long time ago. Note VS Code doesn't use the webgl renderer yet so it could be related to that?

ivanwonder · 2019-10-04T14:08:48Z

Stanzilla · 2019-10-04T14:09:34Z

My video about the resizing problem was with the canvas renderer btw

ivanwonder · 2019-10-04T14:13:40Z

@Tyriar it's canvas render. I search the issues and find fixed. but still exist.

Tyriar · 2019-10-04T14:15:35Z

I think this is the line that controls that:

https://github.com/xtermjs/xterm.js/blob/ff790236c1b205469f17a21246141f512d844295/src/browser/renderer/atlas/DynamicCharAtlas.ts#L254

Unless there's a bug in that version of Chrome? 🤷‍♂

Stanzilla · 2019-10-04T14:17:05Z

I just noticed that resizing is fine for all visible lines but off for the ones currently out of view. See https://dl.dropboxusercontent.com/s/7fcwjxyw0yivl86/2019-10-04_16-16-11.mp4

do we have to call .fit() on scroll events?

Tyriar · 2019-10-04T14:31:13Z

I just noticed that resizing is fine for all visible lines but off for the ones currently out of view.

That's just how winpty/conpty work because they wrap lines before xterm.js gets them. With a native pty they let the terminal emulator wrap them, see microsoft/terminal#405

Stanzilla · 2019-10-04T14:33:48Z

Hrm, how do we best handle that then or just have to live with it?

Tyriar · 2019-10-04T14:35:44Z

@Stanzilla you just have to live with it, as you have already been doing if you're on Windows. Until the upstream issue is fixed.

Stanzilla · 2019-10-04T14:40:50Z

meh, okay. I also noticed that resizing duplicates content in this case, looks like this then

ivanwonder · 2019-10-04T14:53:31Z

this is new xterm 4 with chrome 66 in electron
my local chrome version is the latest 77.0.3865.90. 😂

Tyriar · 2019-10-04T15:09:34Z

@ivanwonder oh it's in the demo? I don't see any issues on latest Edge Dev (Chromium 79):

Might be related to the font?

ivanwonder · 2019-10-04T15:16:50Z

DejaVu Sans Mono for Powerline the font I use

LabhanshAgrawal · 2019-10-04T17:55:27Z

I am not facing any text rendering issues with this pr (using canvas renderer). The issue I had mentioned was on merging xterm4 with electron6 branch (which on itself has rendering issues for me)
The only issue in this pr for me is the initial load resize.
(Note: This is all on mac os, can't say about others)

Stanzilla · 2019-10-05T18:09:04Z

Yeah I think we have to ignore the various Windows quirks and accept them. I only have Windows so here, so kinda need you guys to judge if we are otherwise ready or not.

ivanwonder · 2019-10-06T13:06:37Z

It's ok for me. I can live with that.

GitSquared · 2019-10-06T13:51:40Z

Everything works great for me on Linux, however the "About" pop-up shows the renderer is set to canvas - I'm not sure if that's expected?
Latest Hyper stable release defaults to WebGL instead.

LabhanshAgrawal · 2019-10-06T13:58:29Z

@GitSquared WebGL is disabled in this pr because it had some issues. See line 139 in term.js

GitSquared · 2019-10-06T14:04:16Z

@LabhanshAgrawal Oh OK, got it. I wrote this comment but didn't think it was copied here.

Stanzilla · 2019-10-06T14:35:27Z

Are we ready to merge then?

LabhanshAgrawal · 2019-10-06T18:19:10Z

There is the initial load resize issue, which is small and the fixing of webgl renderer, both can be tracked in new issues. I think it should be ok to merge now.

Stanzilla added 4 commits September 23, 2019 19:53

update xterm to 4

5438c7f

add back webGL addon

978ee1f

remove ligatures xterm addon, unused atm and does not work with webGL…

94a66c7

… renderer

update node-pty

5ab5069

Stanzilla added the WIP label Sep 23, 2019

Stanzilla added 2 commits September 24, 2019 16:45

more work on trying to get xterm4 to work

5cb06db

update xterm to 4.0.2

ee48b03

move where xterm addon props are assigned

37d7f26

more updates

add0934

LabhanshAgrawal reviewed Sep 24, 2019

View reviewed changes

Comment thread lib/components/term.js Outdated

only load webgl addon if webgl is enabled

0686411

fix search function

66060e5

remove rendererType setting, the xterm addon does this for us

59bf928

use xcode 10.3 on circleci

917efab

This was referenced Oct 6, 2019

Issue with resizing during initial load #3848

Closed

Work on re-enabling WebGL renderer #3849

Closed

Stanzilla merged commit 2cc4c9e into vercel:canary Oct 6, 2019

Stanzilla deleted the xterm4 branch October 6, 2019 18:52

GitSquared mentioned this pull request Oct 9, 2019

Add support for font ligatures via xterm-addon-ligatures - Close #3607 #3853

Merged

LabhanshAgrawal mentioned this pull request Nov 24, 2019

remove unused onURLAbort prop #4002

Merged

Conversation

Stanzilla commented Sep 23, 2019

Uh oh!

Stanzilla commented Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Stanzilla commented Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LabhanshAgrawal commented Sep 24, 2019

Uh oh!

LabhanshAgrawal commented Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Stanzilla commented Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Stanzilla commented Sep 24, 2019

Uh oh!

LabhanshAgrawal commented Sep 24, 2019

Uh oh!

Stanzilla commented Sep 24, 2019

Uh oh!

LabhanshAgrawal commented Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Stanzilla commented Sep 24, 2019

Uh oh!

LabhanshAgrawal commented Sep 24, 2019

Uh oh!

LabhanshAgrawal commented Sep 24, 2019

Uh oh!

Stanzilla commented Sep 24, 2019

Uh oh!

LabhanshAgrawal commented Sep 24, 2019

Uh oh!

Stanzilla commented Sep 24, 2019

Uh oh!

LabhanshAgrawal commented Sep 24, 2019

Uh oh!

Stanzilla commented Oct 4, 2019

Uh oh!

Stanzilla commented Oct 4, 2019

Uh oh!

ivanwonder commented Oct 4, 2019

Uh oh!

Tyriar commented Oct 4, 2019

Uh oh!

ivanwonder commented Oct 4, 2019

Uh oh!

Stanzilla commented Oct 4, 2019

Uh oh!

ivanwonder commented Oct 4, 2019

Uh oh!

Tyriar commented Oct 4, 2019

Uh oh!

Stanzilla commented Oct 4, 2019

Uh oh!

Tyriar commented Oct 4, 2019

Uh oh!

Stanzilla commented Oct 4, 2019

Uh oh!

Tyriar commented Oct 4, 2019

Uh oh!

Stanzilla commented Oct 4, 2019

Uh oh!

ivanwonder commented Oct 4, 2019

Uh oh!

Tyriar commented Oct 4, 2019

Uh oh!

ivanwonder commented Oct 4, 2019

Uh oh!

LabhanshAgrawal commented Oct 4, 2019

Uh oh!

Stanzilla commented Oct 5, 2019

Uh oh!

ivanwonder commented Oct 6, 2019

Uh oh!

GitSquared commented Oct 6, 2019

Stanzilla commented Sep 23, 2019 •

edited

Loading

Stanzilla commented Sep 24, 2019 •

edited

Loading

LabhanshAgrawal commented Sep 24, 2019 •

edited

Loading

Stanzilla commented Sep 24, 2019 •

edited

Loading

LabhanshAgrawal commented Sep 24, 2019 •

edited

Loading