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

Replace vscode built-ins by extension pack #93

Merged
merged 3 commits into from
Sep 3, 2021
Merged

Conversation

marcdumais-work
Copy link
Contributor

@marcdumais-work marcdumais-work commented May 12, 2021

What it does

This PR removes most of the vscode built-ins, that were previously listed individually, and replaces them with the built-in extensions pack, that we recently produced. That pack contains all built-ins, so Blueprint will now provide, OOTB, provide very similar feature as VS Code, including grammars that enable syntax highlight in many languages and e.g. themes.

There is one extension ATM that does not work as expected from the pack, that we pin to a specific version known to work better: vscode.git, and one extension that we exclude: vscode.extension-editing.

update: we added a second one, vscode.markdown-language-features, that we now pin to an earlier version, that's known to work well.

Also included in this PR is the switch from the Theia-specific git extension, @theia/git, to the vscode equivalent, vscode.git/vscode.git-ui. This brings Blueprint closer to VS Code, and enables using the very popular extension eamodio/vscode-gitlens.

Known problems

I think the below issues are now addressed - I'm keeping them for reference.

  • Currently, the extensions, that are part of a pack, that's itself listed in the app's package.json, are resolved at runtime, when the user first starts the app. It takes a minute or two for all built-ins to download, and everything falls back on its feet.
    • We are working on a mechanism to optionally resolve extension packs at buildtime.
    • done - extensions part of a pack are resolved at built-time
  • contrary to my expectation, a very recent version of built-ins, pulled through the pack, gets pulled: 1.57.0-next.0988e056b2d. I expected a version that about matches our vscode extensions API level, ~1.50.0 IIRC. Maybe some recent improvements are still on master, not part of @theia v1.13.0 that this draft PR currently uses.
  • extensions that are explicitly mentioned in package.json, like markdown-language-features are ignored at runtime, in favor of the version fetched for the pack. Maybe it's a matter of activation order? e.g. last activated wins? For whatever reason, the pack version wins, which breaks e.g. markdown language features
    • fixed - the issue was the label provided along with the extensions. It needs to match exactly publisher.id, else it will unzip in a different folder and it's a matter of luck which version is picked at startup.
  • in packaged form, the built-in extensions pack currently installs the extensions under /tmp/vscode-unpacked. This means the extensions will potentially be installed more than onc[WIP] re
    • no longer the case since the extensions part of a pack are resolved at built-time - they end-up in the plugins folder, in an unzipped form.
  • a few extensions do not currently behave correctly. We might need an exclusion mechanism, to black-list extensions that currently cause problems, so they are ignored.
    • not less needed ATM since we found working versions for all builtins (a few are pinned to older versions than the pack)
  • Until git: consider switching to builtin extensions #61 is done, the built-in git extension will potentially clash with Theia's
    • update: as suggested, I included this change in this PR. This takes care of the clash - it's now possible to install and use Gitlens

How to test

Build packages and verify that everything seem to work as well as before. Extensions that were known to be "sensitive" and that may benefit from a bit more tests are the ones that handle json and markdown files.

To confirm that more features are now available through the vscode built-in extensions pack, you can try to switch theme, using command preferences: color theme, and chose e.g. "dark (visual studio)".

Review checklist

Reminder for reviewers

@marcdumais-work marcdumais-work changed the title Builtin pack [WIP] replace vscode built-ins by extension pack May 12, 2021
@marcdumais-work marcdumais-work force-pushed the builtin-pack branch 2 times, most recently from 7f43ff5 to b9995ed Compare May 13, 2021 01:13
@marcdumais-work marcdumais-work force-pushed the builtin-pack branch 5 times, most recently from 827d7b9 to bb168db Compare May 17, 2021 18:21
@marcdumais-work marcdumais-work added this to the Q2 2021 milestone May 17, 2021
@vince-fugnitto
Copy link
Member

I confirmed (from bb168db) that the following pull-request eclipse-theia/theia#9486 fixes the issue related to vscode-builtins pulling versions which exceed the version specified by the API version (we should fetch the version that matches our API for vscode-builtins for the same reasons mentioned in the pull-request).

I verified for both:

  • blueprint started with yarn start.
  • blueprint packaged with yarn package (AppImage).

I verified with the best way I knew how, to reference my pull-request locally by updating:

"@theia/vsx-registry": "1.12.1",

to: (local dependency reference specific to my environment)

"@theia/vsx-registry": "file:../../../theia/packages/vsx-registry/",

blueprint-builtin-1 50

@marcdumais-work
Copy link
Contributor Author

Thanks @vince-fugnitto . So we should be is pretty good shape, once we merge your referenced PR. Hopefully in time for theia 1.14.0 next week.

@marcdumais-work
Copy link
Contributor Author

I will soon push a version with an updated yarn.lock, to benefit from this recently merged, relevant PR

@marcdumais-work marcdumais-work force-pushed the builtin-pack branch 3 times, most recently from 1d8764e to 0eab37b Compare August 13, 2021 20:10
@marcdumais-work
Copy link
Contributor Author

I expect #157 will make it possible to exclude the one currently known misbehaving vscode built-in extension (vscode.extension-editing), pulled by the pack, making this PR finally merge-able.

@marcdumais-work marcdumais-work force-pushed the builtin-pack branch 6 times, most recently from fb17571 to 29843a3 Compare September 1, 2021 19:06
@marcdumais-work marcdumais-work changed the title [WIP] replace vscode built-ins by extension pack Replace vscode built-ins by extension pack Sep 1, 2021
@marcdumais-work marcdumais-work marked this pull request as ready for review September 1, 2021 20:39
@jfaltermeier
Copy link
Contributor

Looks good to me. Git-Support, Json-Editing, Java, Typescript all worked fine in my small test.
One thing that surprised me was that the default association for markdown files was the preview for me. Is this expected?
Editing markdown files with Open with -> Code Editor works fine however.

@marcdumais-work
Copy link
Contributor Author

Editing markdown files with Open with -> Code Editor works fine however.

That is a bit strange. So far, I can only open markdown source by explicitly doing "open-with -> code editor". I will investigate.

@marcdumais-work
Copy link
Contributor Author

@jfaltermeier Markdown should behave better - I have pinned an older version that's known to work well. Before, we had two extensions providing markdown support - I have removed the one that's Theia-specific, so the one from vscode is always used. The removed one used to provide the "open-with" entries when an .md file was selected.

We now have an extension pack, that lists all vscode-builtin
extensions.

As a first step, let's aim to have more features than
not, and use that pack. For the most part, extensions
listed in a pack should be installed, at the latest
version that is compatible with the currently used Theia
framework version, wr to the vscode extensions API. We
need not manually update the version of the built-in
extensions, any more.

A few of the vscode built-in extensions do not work well,
with the version pulled by the pack. These we need to pin to a
specific version, that's known to work, using the old way.
e.g.:
`vscode.git` requires v1.52.1 instead of v1.50.0

Closes #26

Signed-off-by: Marc Dumais <[email protected]>
Fixes #61

This will permit enhanced vscode compatibility and make popular
extension GitLens usable in Blueprint.

Signed-off-by: Marc Dumais <[email protected]>
It behaves better. Also removed Theia-specific extension that provides
markdown rendering: @theia/preview : keeping both is confusing

Signed-off-by: Marc Dumais <[email protected]>
@marcdumais-work
Copy link
Contributor Author

I have rebased to the latest master branch, to benefit from Theia v1.17.2.

Copy link
Contributor

@jfaltermeier jfaltermeier left a comment

Choose a reason for hiding this comment

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

👍

@marcdumais-work
Copy link
Contributor Author

Thanks for the review @jfaltermeier

@marcdumais-work marcdumais-work merged commit 88ba1cc into master Sep 3, 2021
@marcdumais-work marcdumais-work deleted the builtin-pack branch September 3, 2021 15:22
@marcdumais-work
Copy link
Contributor Author

marcdumais-work commented Sep 7, 2021

@brianking With this PR merged, we now have the full set of vscode built-in extensions (minus one or two we explicitly exclude), bundled in Blueprint. This is a nice milestone, feature-wise, I think.

This means we now have:

  • a wide set of grammars, covering a wide array of programming and scripting languages, that will automatically provide "syntax highlighting"
    e.g. Windows/DOS batch files (.bat):
    image

  • Various new themes are now available, accessible from the command palette: "preferences: color theme"
    image
    e.g. "Red":
    image
    "Abyss":
    image

See here, under section "bundled extensions", for the full list of built-in extensions that are part of the pack: https://open-vsx.org/extension/eclipse-theia/builtin-extension-pack/1.50.1

All this good content is available from the download page, version 1.17.2 and later.

@brianking
Copy link

@marcdumais-work This is great, and something we should be making noise about.

Would it make sense to wait for Beta 2 to be released, then we can do a blog post and / or some social posts?

@koegel
Copy link

koegel commented Sep 8, 2021

@marcdumais-work This is great, and something we should be making noise about.

Would it make sense to wait for Beta 2 to be released, then we can do a blog post and / or some social posts?

Yes, it would make sense to wait for the beta 2 release imho. @jfaltermeier is planning on building and publishing the release on September 21st. It will also contain some interesting fixes for the update function.

@brianking
Copy link

Yes, it would make sense to wait for the beta 2 release imho. @jfaltermeier is planning on building and publishing the release on September 21st. It will also contain some interesting fixes for the update function.

Was Beta 2 officially released?

@koegel
Copy link

koegel commented Oct 7, 2021

Sorry for the late reply, you probably know by now but just for the sake of completeness:
https://eclipsesource.com/blogs/2021/09/27/eclipse-theia-blueprint-beta-2-is-released/

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.

5 participants