-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
add java-debug extension based on vscode-java-debug #3613
Conversation
db66fe4
to
f355b4f
Compare
It is ready for the review. I've tested with: https://github.com/kasecato/vscode-javadebug-sample |
663729d
to
bc44d52
Compare
I am reviewing it now... (I am checking the browser version only: #3693) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! worked nicely for me.
I cannot run on Windows:
|
Although it is just a guess, the problem could be here: https://github.com/theia-ide/theia/pull/3613/files#diff-70a46c1aaadf49045db1097bc9e1f699R89 |
child.bind(MessagingContainer).toConstantValue(container); | ||
child.bind(MessagingContribution).toSelf(); | ||
return child.get(MessagingContribution); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be singleton
@@ -54,7 +54,7 @@ | |||
"name": "Launch Backend", | |||
"program": "${workspaceRoot}/examples/browser/src-gen/backend/main.js", | |||
"args": [ | |||
"--log-level=debug", | |||
"--hostname=0.0.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hello, I don't see how this change it is related to this feature ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is related because I needed it to debug extensions
@@ -1,5 +1,6 @@ | |||
{ | |||
"rules": { | |||
"deprecation": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hello, I don't see how this change it is related to this feature ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've deprecated some APIs and want other developers to notice it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my point is why deprecated notice is not discussed first
and why it should be deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, let's discuss deprecation on Tuesday
btw it is just warning, it won't break builds and so on. There is also no APIs broken in core and developers can stop using moved APIs over time.
@@ -94,7 +91,7 @@ export class MessageClient { | |||
* To be implemented by an extension, e.g. by the messages extension. | |||
*/ | |||
showProgress(progressId: string, message: ProgressMessage, cancellationToken: CancellationToken): Promise<string | undefined> { | |||
this.logger.info(message.text); | |||
console.info(message.text); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all console calls are implemented by logger already, there is no need to inject Logger
hello, why it is not in a separate repository ? |
True, but we should move everything at once. |
@akosyakov AFAIK plug-in extension is only using |
@benoitf ok, it was not changed |
fbbad1c
to
47692a4
Compare
@kittaakos I've addressed all your comments. |
@kittaakos thank you |
@marcdumais-work CQs:
|
The CQs looks good, thanks Anton. We now need to wait until the Eclipse Foundation has the time to look at them. In many cases they will give "parallel IP" check-in permission, if, after a summary look, the Foundation thinks the CQ does not raise red flags. When we have that permission for both CQs, we can merge. |
Update on the CQs: no longer blocking the merge of this PR: 18453: license_certified |
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
47692a4
to
b7b0e1b
Compare
TODO:
Debugging:
HCR:
Code lenses: