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.

Sign up for GitHub

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

Jump to bottom

Stub vscode 'Notebook' API #57

Closed
wants to merge 2 commits into from
Closed

Stub vscode 'Notebook' API #57

wants to merge 2 commits into from

Conversation

rschnekenbu
Copy link

What it does

Stub vscode 'Notebook' API
This enables the possibility to load and start plugins which depend on the 'Notebook' API. The 'Notebook' functionality will not be operational but the rest of the functionality offered by the plugin will be.

Fixes eclipse-theia#11786

Contributed on behalf of ST Microelectronics

How to test

  1. Install extension GitHub Issue Notebooks from open-vsx, by downloading the vsix file and installing the downloaded file from extension view (note: tested on 0.0.127 on my side).
  2. Reproduce the problem in the theia instance before switching to this PR. Try to create a new Github Issues notebook (ctrl+shift+p and type New Github Issue Notebook)
  3. An error should be displayed: Activating extension 'GitHub Issue Notebooks' failed: vscode__WEBPACK_IMPORTED_MODULE_0__.workspace.onDidOpenNotebookDocument is not a function
  4. Update your theia repository to use this PR
  5. Open the same workspace and make sure the message above is no longer displayed when creating a new Github issues notebook. Nothing should happen, as notebook API is only stubbed, not implemented.

Review checklist

Reminder for reviewers

@rschnekenbu rschnekenbu changed the title Issues/11786 Stub vscode 'Notebook' API Nov 29, 2022
tsmaeder
tsmaeder reviewed Dec 5, 2022
View reviewed changes
packages/plugin-ext/src/plugin/plugin-context.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/plugin/plugin-context.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/plugin/file-system-ext-impl.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/common/plugin-api-rpc.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/plugin/plugin-context.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/plugin/plugin-context.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/plugin/types-impl.ts Show resolved Hide resolved
packages/plugin-ext/src/plugin/types-impl.ts Show resolved Hide resolved
packages/plugin-ext/src/plugin/workspace.ts Outdated Show resolved Hide resolved
@rschnekenbu
Copy link
Author

@tsmaeder, I updated the branch based on your comments for a new review.
The build issue seems common to other PR in the official theia repository.

@tsmaeder
Copy link
Collaborator

tsmaeder commented Dec 7, 2022

Hi @rschnekenbu I added two new comments. The rest looks fine to me now, but I can't mark the comments as resolved for some reason 🤷

@tsmaeder
Copy link
Collaborator

tsmaeder commented Dec 7, 2022

In the current version, you get an error in the browser log saying "notebook api is stubbed". I believe this is correct and before the most recent changes, one would have seen an exception for access to "undefined".

@rschnekenbu
Copy link
Author

The 2 remaining comments have been adressed in 3rd commit.
I do have the same error thrown in the browser log, as expected.

As a note: With the current patch, the theia vscode API coverage status report indicates all notebook related APIs to be stubbed, except the ones from the Tab API. The tab API is currently not implemented at all.
notebook-stubbed

@tsmaeder
Copy link
Collaborator

tsmaeder commented Dec 7, 2022

LTGM now.

@rschnekenbu
Stub vscode 'Notebook' API
9d5f79c 
This enables the possibility to load and start plugins which depend on the 'Notebook' API.  The 'Notebook' functionality will not be operational but the rest of
the functionality offered by the plugin will be.

Contributed on behalf of STMicroelectronics

Signed-off-by: Remi SCHNEKENBURGER <rschnekenburger@eclipsesource.com>
@rschnekenbu
Update CHANGELOG.md
6e7d8a3 
update changelog with PR information
@rschnekenbu
Copy link
Author

closing as main PR 11993 has been merged on main eclipse-theia repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsmaeder tsmaeder tsmaeder approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[vscode] Stub vscode 'NoteBook' API
2 participants
@rschnekenbu @tsmaeder