Implement missing API for vscode.typescript-language-features@1.62.3 #11083

colin-grant-work · 2022-04-27T14:26:44Z

What it does

Fixes #11129 by updating the MarkdownString implementation available to plugins.
Fixes #10819, fixes #10979, fixes #11036

Registered CQ

This PR implements missing API required by recent versions of the TypeScript plugin. In particular:

LanguageStatusItem and related API (vscode: support createLanguageStatusItem API #10979 & Support vscode.LanguageStatusItem #11036)

This required a significant reworking of the StatusBar, and much of this is copied or adapted from VSCode.

Support for affinity between two items

Support for MarkdownStrings and HTML elements in hovers

Improvement in MarkdownString types and implementations

TextDocumentChangeReason

How to test

TS Plugin Activation, Language Server

Download the TS plugin and TS language features plugin and replace the corresponding entries in your theia/plugins folder.
Start the application with Theia as workspace and open a TypeScript file.
Observe that you do not see any toasts about the plugin failing to activate.
Use some new features, e.g. find an override modifier and ctrl+click to go to the super method. Observe that override is not treated as a syntax error.

Language Status Item (and HTML-element tooltips in StatusBar)

With a TS file open, observe that you get a language status item ({}) next to the language indicator.
Hover over that, and observe that two items are available.
Clicking the links should execute the associated commands to select TS version or open the tsconfig file for the current editor.
Clicking the pins should add / remove that command to / from the status bar as an independent item, and clicking that item should execute the command.
The pinned items should appear and disappear according to the language of the current editor.

Markdown Rendering (MarkdownString in tooltips in StatusBar)

Put markdown-renderer-test-0.0.1.zip in your plugins folder and start the application.
Start the application.
Run the command VSCode API: Show status item with Markdown tooltip
A status item should appear on the left side with the text 'Check my tooltip'
Hover over that item, and you should see

The gear should spin, and if you have an editor open as the current widget, clicking the link should trigger the 'save-as' action for that editor.

Review checklist

As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

As a reviewer, I agree to behave in accordance with the review guidelines

planger · 2022-04-28T07:54:32Z

@colin-grant-work I saw that there's a playwright test that consistently fails with this PR.

[playwright]     > 43 |         expect(await notificationIndicator.isVisible()).toBe(true);
[playwright]          |                                                         ^
...
[playwright]   1 failed
[playwright]     ../../src/tests/theia-status-bar.test.ts:38:9 › Theia Status Bar › should contain status bar elements 
[playwright]   1 skipped
[playwright]   49 passed (2m)

Looks like the selector for the notification indicator doesn't work anymore.
Just wanted to offer to look into it, if you want. Feel free to skip this test and merge this PR, once it is ready, and create an issue for us to fix and re-enable the test.

msujew · 2022-04-28T09:44:12Z

I'll look into this once I'm back from vacation in mid May.

colin-grant-work · 2022-04-28T13:49:32Z

@jfaltermeier, thanks for checking. I implemented a service that renders tooltips on status bar items as HTML elements rather than using the title property, but it looks like some of the Playwright machinery to determine whether a given status bar item is visible checks the title. At the moment, we don't put much other metadata on those elements, but we could for example add an id property that would make it easy to pick out one status bar item among the rest?

planger · 2022-04-29T11:18:26Z

@colin-grant-work Thanks for the feedback!

I think we currently use either icons or titles as selectors for notifications or status bar items in general. Originally, because there wasn't other metadata and it's the closest to what the user would use to identify a status bar item.

But id or data-attributes would be fine too and it would be very low effort to change that in the page objects.
If you could add those additional attributes, that'd be great. Otherwise I'm happy to add them too, when I fix up the page objects. As you prefer.

Thanks!

vince-fugnitto · 2022-05-09T16:44:51Z

The CQ opened for this pull-request has been approved 👍

msujew

Looks great! Works as expected and the vscode API is correctly supported as far as I can tell.

I have a few minor issues and a suggestion regarding the hover API that you introduced.

packages/core/src/browser/status-bar/status-bar-hover-manager.ts

packages/core/src/browser/status-bar/status-bar-types.ts

packages/editor/src/browser/language-status/editor-language-status-service.ts

packages/core/src/browser/status-bar/status-bar-hover-manager.ts

packages/editor/src/common/paths.ts

colin-grant-work · 2022-05-17T00:03:24Z

@planger, I have added an id to status bar elements and reworked the corresponding page objects. I ended up modifying them rather a lot because finding things by id is much easier than by icon or by title, but it may be desirable to retain some of the old icon-related functionality. Let me know what you think.

planger · 2022-05-17T07:08:01Z

@planger, I have added an id to status bar elements and reworked the corresponding page objects. I ended up modifying them rather a lot because finding things by id is much easier than by icon or by title, but it may be desirable to retain some of the old icon-related functionality. Let me know what you think.

Very nice! Thank you!

You are certainly right, working with IDs makes the code much leaner. I think the use case for selecting by icons is rather uncommon for such rather static core elements, like status bar items. So I think only using the ID is completely fine for now.

Thanks again!

vince-fugnitto

I'll continue with a functional review, I only have some minor comments.

examples/api-tests/src/preferences.spec.js

examples/playwright/src/theia-notification-indicator.ts

packages/callhierarchy/src/common/glob.ts

packages/callhierarchy/src/common/paths.ts

packages/core/src/browser/common-frontend-contribution.ts

packages/core/src/browser/status-bar/status-bar.tsx

packages/editor/src/browser/language-status/editor-language-status-service.ts

vince-fugnitto · 2022-05-17T16:05:36Z

packages/editor/src/browser/language-status/editor-language-status.css

+}
+
+.hover-language-status>.element>.left>.detail:not(:empty)::before {
+    /* endash */


I might be wrong but isn't it spelled like so: ;)

Suggested change

/* endash */

/* en dash */

Compromise on en-dash?

packages/editor/src/common/language-selector.ts

msujew

Thanks, the changes look good to me!

The updated versions of the typescript extensions start as expected
Markdown rendered hovers behave as in vscode
Status bar elements continue to work as expected

vince-fugnitto

The changes work well for me 👍

I confirmed that:

existing statusbar items contributed by the framework look and work as expected
the typescript-language-server builtin works without errors
- the { } statusbar item looks good and works
- the { } pin/unpin functionality works
- the { } config file works
the contributed statusbar item works in markdown, icons are properly displayed

…feature/api-for-ts Conflicts: packages/plugin-ext/src/common/plugin-api-rpc.ts packages/plugin-ext/src/main/browser/languages-main.ts packages/plugin-ext/src/plugin/languages.ts packages/plugin-ext/src/plugin/plugin-context.ts packages/plugin/src/theia.d.ts

colin-grant-work added typescript issues related to the typescript language vscode issues related to VSCode compatibility CQ Required (deprecated) issues requiring a CQ (contributor questionnaire) labels Apr 27, 2022

msujew self-requested a review April 28, 2022 09:44

This was referenced May 11, 2022

[vscode] Support optional property MarkdownString#supportThemeIcons #11129

Closed

Call onHide in MonacoContextMenuService #11152

Merged

colin-grant-work force-pushed the feature/api-for-ts branch from 9b7ae66 to 69f0381 Compare May 13, 2022 16:57

msujew reviewed May 16, 2022

View reviewed changes

colin-grant-work force-pushed the feature/api-for-ts branch from 69f0381 to 3c6680c Compare May 16, 2022 20:19

Implement missing API for vscode.typescript-language-features@1.65.3

fed00c9

colin-grant-work force-pushed the feature/api-for-ts branch from 3c6680c to fed00c9 Compare May 16, 2022 20:37

Partial MS feedback

42d0058

Fix Playwright tests

003b217

colin-grant-work force-pushed the feature/api-for-ts branch from 3d72edf to 003b217 Compare May 17, 2022 14:09

Move paths and glob to core

1ed70d2

vince-fugnitto reviewed May 17, 2022

View reviewed changes

msujew approved these changes May 17, 2022

View reviewed changes

colin-grant-work added 4 commits May 17, 2022 10:45

VF comments

55beb3d

Fix toJSON

69c9d50

Update tests

d51622b

A test of code actions

cb05aab

vince-fugnitto approved these changes May 17, 2022

View reviewed changes

Remove code action test & plugin version update for Node 12

0fad834

colin-grant-work merged commit 70466bb into eclipse-theia:master May 18, 2022

colin-grant-work added this to the 1.26.0 milestone May 18, 2022

colin-grant-work deleted the feature/api-for-ts branch May 18, 2022 21:21

colin-grant-work mentioned this pull request May 25, 2022

upgrade to latest VS Code built-in extensions #7712

Closed

colin-grant-work mentioned this pull request Jun 9, 2022

Refactor StatusBarHoverManager for general use #11281

Closed

msujew mentioned this pull request Nov 15, 2022

Generic hover service support #11869

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement missing API for vscode.typescript-language-features@1.62.3 #11083

Implement missing API for vscode.typescript-language-features@1.62.3 #11083

colin-grant-work commented Apr 27, 2022 •

edited

Loading

planger commented Apr 28, 2022

msujew commented Apr 28, 2022

colin-grant-work commented Apr 28, 2022 •

edited

Loading

planger commented Apr 29, 2022

vince-fugnitto commented May 9, 2022

msujew left a comment

colin-grant-work commented May 17, 2022

planger commented May 17, 2022

vince-fugnitto left a comment

vince-fugnitto May 17, 2022

colin-grant-work May 17, 2022 •

edited

Loading

msujew left a comment

vince-fugnitto left a comment

Implement missing API for vscode.typescript-language-features@1.62.3 #11083

Implement missing API for vscode.typescript-language-features@1.62.3 #11083

Conversation

colin-grant-work commented Apr 27, 2022 • edited Loading

What it does

How to test

TS Plugin Activation, Language Server

Language Status Item (and HTML-element tooltips in StatusBar)

Markdown Rendering (MarkdownString in tooltips in StatusBar)

Review checklist

Reminder for reviewers

planger commented Apr 28, 2022

msujew commented Apr 28, 2022

colin-grant-work commented Apr 28, 2022 • edited Loading

planger commented Apr 29, 2022

vince-fugnitto commented May 9, 2022

msujew left a comment

Choose a reason for hiding this comment

colin-grant-work commented May 17, 2022

planger commented May 17, 2022

vince-fugnitto left a comment

Choose a reason for hiding this comment

vince-fugnitto May 17, 2022

Choose a reason for hiding this comment

colin-grant-work May 17, 2022 • edited Loading

Choose a reason for hiding this comment

msujew left a comment

Choose a reason for hiding this comment

vince-fugnitto left a comment

Choose a reason for hiding this comment

colin-grant-work commented Apr 27, 2022 •

edited

Loading

colin-grant-work commented Apr 28, 2022 •

edited

Loading

colin-grant-work May 17, 2022 •

edited

Loading