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

[vscode] semantic highlighting support #8593

Merged
merged 2 commits into from
Nov 2, 2020

Conversation

akosyakov
Copy link
Member

@akosyakov akosyakov commented Oct 6, 2020

What it does

How to test

  • start theia
  • open application-shell.ts
  • observe how imports are colored before ts smartness is ready and after
    • you can see whether ts smartness initialization progress in the status bar
  • open preferences side by side with the editor, find semantic highligting toggle, use it and see how highlighting is changing in the editor

Review checklist

Reminder for reviewers

@akosyakov akosyakov added languages issue related to languages vscode issues related to VSCode compatibility labels Oct 6, 2020
@akosyakov akosyakov added the CQ Required (deprecated) issues requiring a CQ (contributor questionnaire) label Oct 6, 2020
@akosyakov akosyakov marked this pull request as draft October 6, 2020 09:45
@akosyakov akosyakov force-pushed the akosyakov/activating-extension-7608 branch from 1078bba to d0cd9c7 Compare October 6, 2020 11:53
@akosyakov
Copy link
Member Author

It's turned out that it is required Monaco 0.21.0. Monaco 0.20.0 is not complete. I've patched the theming standalone service for now with fixed from 0.21.0. We should remove the patch after next migration.

@akosyakov akosyakov marked this pull request as ready for review October 6, 2020 11:55
Copy link
Contributor

@JanKoehnlein JanKoehnlein left a comment

Choose a reason for hiding this comment

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

LGTM.

@kittaakos
Copy link
Contributor

@akosyakov,

  • Do you know if the current changeset will be (fully) compatible with the LSP-based semantic-highlighting out of the box once it's released? Or will that be available only with monaco 0.22.x or 0.23.x?
  • Can you please recommend another VS Code extension I could use to try out the new feature of this PR?
  • It seems the formatter does not work correctly on your side. Please fix the indentations.

Thank you!

@dunnry
Copy link

dunnry commented Oct 7, 2020

The F# Ionide Extension uses this feature if you want to test it. @kittaakos

@JanKoehnlein
Copy link
Contributor

JanKoehnlein commented Oct 8, 2020 via email

@vince-fugnitto
Copy link
Member

I believe the CQ has now been successfully approved 👍

@akosyakov
Copy link
Member Author

Could someone help with rebasing and merging it?

@vince-fugnitto vince-fugnitto force-pushed the akosyakov/activating-extension-7608 branch from d0cd9c7 to c1a344c Compare October 28, 2020 18:33
@azatsarynnyy
Copy link
Member

I just wonder are we allowed to use the code in an Eclipse repo if the related CQ is still in awaiting_analysis state?
Does that mean it's approved?

@akosyakov
Copy link
Member Author

We only need checkin tag as far as I know. cc @marcdumais-work

@vince-fugnitto
Copy link
Member

I just wonder are we allowed to use the code in an Eclipse repo if the related CQ is still in awaiting_analysis state?
Does that mean it's approved?

@azatsarynnyy I know in the past we proceeded to merge pull-requests when they granted permission, namely the following:

We have completed a high level triage of the attachment and you may now check the content into your project’s source code repository.

I believe at this point they will continue to verify the content (awaiting_analysis), and if an issue arises they will notify us.

@azatsarynnyy
Copy link
Member

Thanks @vince-fugnitto for the explanation!

@vince-fugnitto
Copy link
Member

@akosyakov do we want to merge for today's release?

@akosyakov
Copy link
Member Author

@vince-fugnitto merge when it suites you

@vince-fugnitto vince-fugnitto force-pushed the akosyakov/activating-extension-7608 branch from c1a344c to 91c1de9 Compare October 30, 2020 18:16
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I reviewed the changes and they work well 👍
I'll merge and if there are issues we have the month to resolve any of them.

@vince-fugnitto vince-fugnitto merged commit 9393b54 into master Nov 2, 2020
@vince-fugnitto vince-fugnitto deleted the akosyakov/activating-extension-7608 branch November 2, 2020 14:05
@github-actions github-actions bot added this to the 1.8.0 milestone Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CQ Required (deprecated) issues requiring a CQ (contributor questionnaire) languages issue related to languages vscode issues related to VSCode compatibility
Projects
None yet
6 participants