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

9012-registerTerminalLinkProvider-stub: for vscode-java-debug #9048

Merged

Conversation

danarad05
Copy link
Contributor

@danarad05 danarad05 commented Feb 10, 2021

Signed-off-by: Dan Arad dan.arad@sap.com

Added registerTerminalLinkProvider stub which is invoked upon extension activation.

What it does

Fixes first problem of #9012:
vscode-java-debug invokes vscode.window.registerTerminalLinkProvider upon extension activation. This method was not implemented in Theia - added stubs.
For second problem see #9049

How to test

  1. Open in Theia java project
  2. After load you can run or debug project
    image

Approved CQ (existing already) covering theia.d.ts copied code: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=22677

Note:
for better behavior change this user setting to "internalConsole" (instead of "integratedTerminal"):
"java.debug.settings.console": "internalConsole"

Review checklist

Reminder for reviewers

@colin-grant-work
Copy link
Contributor

Since this PR contains code copied from VSCode, it may require a CQ, so you may want to get that process started.

@amiramw
Copy link
Member

amiramw commented Feb 10, 2021

Since this PR contains code copied from VSCode, it may require a CQ, so you may want to get that process started.

I checked and this copied code was requested in approved CQ https://dev.eclipse.org/ipzilla/show_bug.cgi?id=22677

@amiramw amiramw changed the title 9012-registerTerminalLinkProvider-stub: in order to facilitate vscode… 9012-registerTerminalLinkProvider-stub: for vscode-java-debug Feb 10, 2021
@colin-grant-work
Copy link
Contributor

I checked and this copied code was requested in approved CQ https://dev.eclipse.org/ipzilla/show_bug.cgi?id=22677

Great! So you read that CQ as permitting copying from VSCode 1.51.0 quite broadly, then? If so, that's very nice, and we should update our wiki entry about copying code from VSCode accordingly.

@danarad05
Copy link
Contributor Author

I checked and this copied code was requested in approved CQ https://dev.eclipse.org/ipzilla/show_bug.cgi?id=22677

Great! So you read that CQ as permitting copying from VSCode 1.51.0 quite broadly, then? If so, that's very nice, and we should update our wiki entry about copying code from VSCode accordingly.

@colin-grant-work
Do I understand that you think we should create a new CQ for this PR and not used that one?

@amiramw
Copy link
Member

amiramw commented Feb 11, 2021

Based on comment by @marcdumais-work in another thread I assume that old CQ that contain the copied code is enough:
#8678 (comment)

packages/plugin/src/theia.d.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/plugin/plugin-context.ts Outdated Show resolved Hide resolved
@marcdumais-work
Copy link
Contributor

To be clear, it's not enough that there exist a CQ for the same vscode version, the CQ attachment has to contain the copied code, verbatim - i.e. all files we copied-from have to be present (we can then modify our version of what we copied, of course).

If we have several CQs per vscode release, we could discuss if it makes sense for the Foundation to analyse vscode versions as they come-out, wholly and preemptively. Then we would not need a CQ if we copy from a pre-approved version.

I assume that old CQ that contain the copied code is enough:
#8678 (comment)

👍

…-java-debug extension added registerTerminalLinkProvider stub which is invoked upon extension activation

Signed-off-by: Dan Arad <dan.arad@sap.com>
@danarad05 danarad05 force-pushed the 9012-registerTerminalLinkProvider-stub branch from 33af8ac to 110afa5 Compare February 14, 2021 10:00
@vince-fugnitto vince-fugnitto added the vscode issues related to VSCode compatibility label Feb 23, 2021
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.

The stub seems fine and it was copied from code covered in the previous CQ:

@amiramw amiramw merged commit 31b0838 into eclipse-theia:master Feb 23, 2021
@paul-marechal paul-marechal added this to the 1.11.0 milestone Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants