microsoft/vscode

Explore to update to Electron 2.0.x

bpasero opened this issue · 74 comments

Hey!

We are in the process of updating VSCode’s Electron version to a new major release (from 1.7.x to 2.0.x). For that reason we would like to get some testers onto this version, which we currently not plan to distribute via insiders but as a separate install that can be run side-by-side with other installs. This install is separate with regards to everything (settings, keybindings, extensions), so it will not touch any of your existing configuration.

Best way of testing is to just use this build for daily development and look out for suspicious things happening 👍 . Visually there should be no difference to stable or insiders (except for the orange icon 🔶).

Builds

Feel free to file issues as you see them, thanks 👍

I created a side-by-side install (with orange icon 🔶!) so that we do not tamper with stable or insiders data. It even supports auto-updates (except for Linux) as we push more changes to our adoption branch. These bits are from last week friday and should be as stable as our insider releases:

Feel free to file issues as you see them, thanks 👍

l0k18 commented

How do I test this thing? I just upgraded to Nvidia 390.42 and this seems to have fixed my X crash issue but I'm keen to help test out this branch. Is there a tag or branch to check out to get these builds? I run Arch so debs and rpms are not relevant to my system.

What's the main difference(s), to look out for?

Works for me, so far, with all my favorite extensions, themes, and user settings.

Where would I copy over snippets etc. from Insiders to this one?

@l0k1verloren this build is from branch https://github.com/Microsoft/vscode/tree/tyriar/electron-2.0.x. Best way to test it is to use it for daily development and report back if you encounter any issues 👍

@smlombardi visually nothing should change really. On Linux, since GTK3 is now used (over GTK2) some file dialogs and menus will likely look different.

You will have to manually copy settings and keybindings over to this build as nothing is shared. Just open the files in one instance and copy the contents over to the other.

l0k18 commented

I switched to the tyriar/electron-2.0.x branch and built and ran it and it seemed normal (exceept nice gtk3 file dialog) but then when it opened a folder nothing rendered and the menus didn't respond.

@l0k1verloren on what Linux distribution are you? can you show a screenshot or video what you see?

l0k18 commented

uname -a Linux monolith 4.15.9-1-ck #1 SMP PREEMPT Tue Mar 13 11:27:38 EET 2018 x86_64 GNU/Linux
Antergos Linux (Arch), fully up to date as at right now.

Initial startup:

image

After opening a (golang) repository folder:

image

+ @Tyriar on this GTK2 vs GTK3 issue if you have any ideas.

@l0k1verloren and this works fine with VS Code stable? Do you see any output in the developer console (Help > Toggle Developer Tools > Console) when you bring up the dialog?

K900 commented

Something is off with the menu bar colors for me - it's white on light grey with the Breeze GTK3 theme.

@K900 this Electron update comes with a change of GTK2 to GTK3, so I am expecting visual changes in native menus and file dialogs. Can you share a screenshot? Does it look different from other application menus?

K900 commented

image

It's not an issue in, say, gtk3-demo:

image

@K900 can you confirm that you are not seeing this with Code stable?

K900 commented

Everything is fine on stable (GTK2), yeah.

image

@K900 have you seen this issue with other applications on your machine (e.g. Chrome)?

@l0k1verloren do you have Chrome installed? I wonder if you could try the file dialog there to see if it fails too (File > Open File).

K900 commented

Chrome doesn't have a menu bar, does it? Firefox does have a menu bar, but it uses the Firefox theme colors. All the GTK3 demo apps use the correct colors.

@K900 yeah good point, how does the context menu in VSCode look like, is it also using bad colors?

K900 commented

The context menu is fine, it's just the menu bar that's using the wrong font color.

@K900 does it also reproduce if you are not using any special theme?

Happy to report it (2018-03-13T14:22:28) seems to work very nicely on Debian testing with Adwaita Dark themed GTK3 and it's nice to see the old GTK2 parts gone. The only minor cosmetic thing I noticed is that the editor background color is dark greyish on lines with text and a few more after file end but then turns black. The same applies to the line number pane. Looking forward to this major update of this excellent editor.

K900 commented

@bpasero what do you mean by "special" theme? Just default Adwaita?

@K900 I thought "Breeze GTK3 theme" was a special theme you enabled

@bjornharrtell can you show me a screenshot of that?

K900 commented

Breeze is just a GTK theme like any other, it's styled to look like the default KDE Breeze style for Qt. I can try it with the default GTK3 theme that GNOME uses.

@K900 yes please. any hints how I can install that theme into Ubuntu 17 or 16?

K900 commented

You should be able to just apt install breeze-gtk IIRC, though it's been a while since I last used Ubuntu on a desktop. I'll test with some other themes later and see what I can find.

@K900 what Linux distribution are you on?

K900 commented

Arch testing, everything up to date. Also, it does look correct (dark background, light text) with the default GTK3 theme, so it might be a Breeze bug.

@basarat :Nice to accept the beta version!

Well……I wanna know more about this (NOT ONLY listed below):

  1. Are you willing to upgrade Nodejs to version 8.9.X? Or have you got a similar plan to publish a new version of re-writing Electron with the big changed version of Nodejs (example: When the specific version of Nodejs isn't maintained, that's the time maybe you will consider upgrade VSC)?

  2. Any other new plans of new features/big changes you wanna add (Including UI/UE (User Experience)/Code Syntanxes……ect) in the future?

@MaleDong this update (Electron 2.0.x) is coming with node.js 8.9.3. As for other plans you can always check our Roadmap.

Looking forward to this update making it into the insiders / stable build because it fixes the font rendering issues I have been facing on Linux (I'm using Ubuntu 18.04). The text is much crisper and I no longer see blue lines between the characters (see screenshots).

I haven't really used this version for doing any work, but the menus look fine.

Exploration version:
exploration
Insiders version:
insiders

@HendrikThePendric the font rendering issue can be traced back to https://bugs.chromium.org/p/skia/issues/detail?id=6663 and I think it has been fixed or at least worked around on electron 1.7.x series so that issue should hopefully go away before vscode migrates to electron 2.x.

@HendrikThePendric @bjornharrtell yes this will come in through the Electron 1.7.12 update that I just merged.

@bpasero I saw insiders is now running on electron 1.7.12. It looks a lot better indeed. Fonts are sharp but colors are still a bit flat. In this electron 2.0 branch the colors are looking much better:
Insiders on electron 1..7.12
after_update
Exploration version
exploration

@HendrikThePendric interesting, I was not aware of any color changes in fonts to be honest.

Looking forward to full gtk3 support. Much better integration with the system. When trying this build I noticed an issue with spacing in dialog boxes.
screenshot from 2018-03-21 11 04 12
I feel the buttons are too large. And Primary and secondary text should be aligned. And maybe use suggested Icon style for Restart button.
Here is an example following elementary HIG (Gnome also follows a similar approach)
image

Finally one more thing I would like to see is csd support.

OS Info:
elementaryOS 0.4 Loki
(Default theme)

@arshubham thanks for the feedback. the dialogs are not under VSCode control though and I suggest you report this over at Electron repository so that we get attention from there: https://github.com/atom/electron

K900 commented

The toolbar coloring issue seems to be fixed for me. Thanks!

l0k18 commented

image

I love the gtk3! :) Though oddly the file/folder open doesn't allow me to create sidebar shortcuts. I think there might be more than a few issues you guys wanna raise with the electrum 2 repo.

I should also just add that my issues with crashing electron apps was completely solved by rolling back my arch distro to the current packages as at december 31. I suspect nvidia but it could also be the kernel causing the X crash issue. Nvidia is the prime suspect for me now, after just fixing a friend's son's gaming machine that had started completely bombing out and restarting during GPU accelerated apps - probably the linux kernel handles the driver fault more gracefully only dropping me back to lightdm but I'm not upgrading my nvidia driver for quite some time now as a result. The issue is very serious and it was distracting me terribly from doing my work. Oh, that machine was a windows machine too, I am very displeased with Nvidia for such shoddy work, it's been just another of the many things that have been really wrong with the first three months of this year. Hopefully 2018 will improve from here on.

Closing, thanks for feedback (still keep it coming if any). We plan to merge this work early in next months sprint.

Reopening as this continues for April via #47289. The exploration builds are running again with Electron 2.0.0-beta.7.

@bpasero Can we expect performance improvements?

l0k18 commented

It seems to be a little faster for me, the version linked to in this thread, and maybe more stable. I have been having some kind of problem with certain electron versions which has been very irritating, causing my X sessions to die. I'm not sure which versions are causing the problem, I have one particular app, Eleos z-coin wallet, that I suspect very highly, I'm going to see if I can update its electron version and maybe the problem will go away, it seems to be the one app that is always running when I have the crashes (and vscode mainline version possibly also)

The build download links are dead, any new ones?

@farid-fari I fixed the links, please try again

Integrated terminal is bugged a bit, the more text it has the slower it gets, To repro it just type in help and try to use it, it becomes very unresponsive. (This was tested on Fedora 27 x64)

@cra0zy and you are not seeing this in Code stable?

/cc @Tyriar

@cra0zy and you are not seeing this in Code stable?

Not seeing it in stable ofc.

This seems to be the Linux manifestation of electron/electron#12042 then.

Tested on another Linux device and the same thing occurs there, so its a global Linux x64 problem it seems.

@cra0zy any difference if you run "code --disable-gpu" from the command line?

None.

@cra0zy thanks for testing, I have extracted this particular into #48193 so that we can keep the discussion focused there.

PR continues via #48290

Is Electron 2.0 supposed to bring capabilities of theming on menu bar and context menu in order to have them dark when using dark theme? If yes, it's not working:
image

l0k18 commented

@Geobert the effect relating to dark themes is for a system running a GTK+3 theme, not for Windows. On windows Electron uses the windows file and other dialogs and windows menus. On Linux this issue of mixed theming is because of the main GUI running GTK+3 but Electron using GTK+2 and is a common issue. There is ways to override the GTK+2 theme with dark but it does not flip on the 'dark theme' toggle, which only affects GTK+3

Thanks for the explanation!

Will this fix the High sierra native tabs issue? electron/electron#10657 (comment)

Hi, could you please update the exploration builds so that they are based on the April release? I would like to use the exploration version to develop an extension with a webview and a dependency that requires node 8. Currently the webview api is missing from these builds and node 8 is missing from the stable release.

@christianvoigt the exploration build is really up to date. It should get updated automatically. What OS are you on?

ah ok. That's strange. Because if I run the webview-sample I get "Running the contributed command: 'catCoding:start' failed." And the Output Log (Extension Host) shows the following error:

[error] TypeError: vscode.window.createWebviewPanel is not a function

Running the webview-sample in my normal VS Code installation works fine. I am on OSX.

Thank you! With the latest build it is working. So for some reason the automatic update did not work I guess.

I downloaded this exploration build and took it for a spin. Specifically, I wanted to check out the native tabs on High Sierra. The Native Tabs functionality is not fixed, even with Electron 2 in the build*.

I tested the contributed phoenixgao build some time ago and it worked. @bpasero I'm recalling some comments over there - is the key here that the Electron package must be built on a High Sierra machine? If so, how can this ever be fixed properly?

High Sierra 10.13.2 (17C88)
Version 1.24.0-exploration (1.24.0-exploration)

  • (not exactly the same as before either, now the native 'Add New Tab' plus sign doesn't show on the far right of the bogus full-width tab).

@johnelm Electron did NOT fix this issue properly. As soon as a new macOS version is released, the issue will show up again. I would suggest to join me in blaming the "fix" here: electron/electron#10657 (comment)

We are building Electron ourselves and have a different build configuration compared to GitHub. As such, the "fix" does not work for us and I mentioned that.

Merged to master.

Reverted this change again, too many bugs and regressions...

@bpasero In my opinion, Would you please think about update to Electron 3.x directly?
Electron 3.0.x is now in beta , I think there is no need to take time to adopt 2.0.x.
Thanks.

@LiPengfei19820619 we're looking at that as well, it's probably a little naive to assume there won't be just as many issues adopting it too though.

@Tyriar Yes, I Know. I don't think it would be an easy work. I appreciate all your job,
As I have been using VS Code for a long time, and I wish VS Code would be better and better.

@bpasero @Tyriar So how do you want to handle this issue differently than the approach of the last iteration?

Merged into master, the latest Insiders is on 2.0.5 #54873