Attention is currently required from: Hyang-Ah Hana Kim.
Patch set 1:Code-Review +2
To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Hyang-Ah Hana Kim.
1 comment:
File src/goDebugFactory.ts:
Patch Set #1, Line 164: // TODO(hyangah): use color distinguishable from the color used from print.
So everything will be one color with this? Which one?
With the legacy adapter we have:
green - console (e.g. adapter logging)
red - stderr
white - stdout
To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Hyang-Ah Hana Kim, Polina Sokolova.
1 comment:
File src/goDebugFactory.ts:
Patch Set #1, Line 164: // TODO(hyangah): use color distinguishable from the color used from print.
So everything will be one color with this? Which one? […]
This prints all of the output with the "console" colors
To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
3 comments:
File src/goDebugFactory.ts:
Patch Set #1, Line 90: printError(
Why does the logging from the extension need to change?
Isn't the logging module also controlling when to print things depending on flags?
How is the old behavior different from the new one here?
Patch Set #1, Line 114: print(`Running: ${dlvPath} ${dlvArgs.join(' ')}`);
Suggest renaming to printToDebugConsole
Patch Set #1, Line 164: // TODO(hyangah): use color distinguishable from the color used from print.
This prints all of the output with the "console" colors
Looks like vscode uses this function to control the colors of output events:
https://github.com/microsoft/vscode/blob/a699ffaee62010c4634d301da2bbdb7646b8d1da/src/vs/workbench/contrib/debug/browser/debugSession.ts#L1127
Do we have access to it from here?
To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Suzy Mueller, Polina Sokolova.
2 comments:
File src/goDebugFactory.ts:
Patch Set #1, Line 90: printError(
Why does the logging from the extension need to change? […]
What do you mean by 'logging from the extension'?
The previously used `goLogging` is an internal logging facility that prints messages to developer console, which isn't meant to be visible to users.
Patch Set #1, Line 164: // TODO(hyangah): use color distinguishable from the color used from print.
So everything will be one color with this? Which one? […]
One color (whatever default determined by the theme).
Unfortunately, I need to look into a way to pick the right color depending on users' color theme.
---
Polina - that's not exposed in vscode api. This is using https://code.visualstudio.com/api/references/vscode-api#DebugConsole
If you want to redirect the debugee's stdout/stderr to stdout/stderr, send them using OutputEvent over the network port instead of depending on dlv's default behavior.
To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.
Patch set 1:Code-Review +2
3 comments:
Patchset:
Let's rename "print" to something more descriptive since you could be printing/logging to different places and then this is good to go.
File src/goDebugFactory.ts:
Patch Set #1, Line 90: printError(
What do you mean by 'logging from the extension'? […]
Ah, I see. I thought it was printing to Debug Console and was controlled by "trace".
Patch Set #1, Line 164: // TODO(hyangah): use color distinguishable from the color used from print.
One color (whatever default determined by the theme). […]
I was hoping it could be exposed through DebugSession. I think for now this is fine. And we can iterate and improve.
To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Suzy Mueller.
2 comments:
File src/goDebugFactory.ts:
Patch Set #1, Line 90: printError(
Ah, I see. I thought it was printing to Debug Console and was controlled by "trace".
Ack
Patch Set #1, Line 114: print(`Running: ${dlvPath} ${dlvArgs.join(' ')}`);
Suggest renaming to printToDebugConsole
Done
To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Suzy Mueller.
Hyang-Ah Hana Kim uploaded patch set #2 to this change.
src/goDebugFactory: redirect stdout/stderr from dlv to Debug Console
Assuming dlv dap keeps using the default debugger Redirect config
debuggee's stdout/stderr is redirected to dlv dap's stdout/stderr
(https://github.com/go-delve/delve/blob/2e80b32c417494ab3fabe929e7051076a7049f7c/pkg/proc/native/proc.go#L333)
Now this CL wires dlv dap's stderr/stdout with
vscode.debug.activeDebugConsole. Currently stdout and stderr
output are not easily distinguishable. We can consider using
different ASCII color codes, but that must be done carefully
because users' color theme may be different.
Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
---
M src/goDebugFactory.ts
1 file changed, 30 insertions(+), 10 deletions(-)
To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Suzy Mueller.
Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/b9695d1e-8453-442c-ad82-3e630ed692fe
Patch set 2:TryBot-Result +1
Hyang-Ah Hana Kim submitted this change.
src/goDebugFactory: redirect stdout/stderr from dlv to Debug Console
Assuming dlv dap keeps using the default debugger Redirect config
debuggee's stdout/stderr is redirected to dlv dap's stdout/stderr
(https://github.com/go-delve/delve/blob/2e80b32c417494ab3fabe929e7051076a7049f7c/pkg/proc/native/proc.go#L333)
Now this CL wires dlv dap's stderr/stdout with
vscode.debug.activeDebugConsole. Currently stdout and stderr
output are not easily distinguishable. We can consider using
different ASCII color codes, but that must be done carefully
because users' color theme may be different.
Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/296930
Trust: Hyang-Ah Hana Kim <hya...@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hya...@gmail.com>
TryBot-Result: kokoro <noreply...@google.com>
Reviewed-by: Suzy Mueller <suz...@golang.org>
Reviewed-by: Polina Sokolova <pol...@google.com>
---
M src/goDebugFactory.ts
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/src/goDebugFactory.ts b/src/goDebugFactory.ts
index 28d0ff8..c6b8e16 100644
--- a/src/goDebugFactory.ts
+++ b/src/goDebugFactory.ts
@@ -7,7 +7,6 @@
import { ChildProcess, ChildProcessWithoutNullStreams, spawn } from 'child_process';
import * as fs from 'fs';
import { DebugConfiguration } from 'vscode';
-import { logError, logInfo } from './goLogging';
import { envPath } from './utils/pathUtils';
import { killProcessTree } from './utils/processUtils';
import getPort = require('get-port');
@@ -69,18 +68,14 @@
} else {
configuration.port = await getPort();
}
- const dlvDapServer = spawnDlvDapServerProcess(configuration, logInfo, logError);
+ const dlvDapServer = spawnDlvDapServerProcess(configuration);
// Wait to give dlv-dap a chance to start before returning.
return await new Promise<{ port: number; host: string; dlvDapServer: ChildProcessWithoutNullStreams }>((resolve) =>
setTimeout(() => resolve({ port: configuration.port, host: configuration.host, dlvDapServer }), 500)
);
}
-function spawnDlvDapServerProcess(
- launchArgs: DebugConfiguration,
- logFn: (...args: any[]) => void,
- logErrFn: (...args: any[]) => void
-) {
+function spawnDlvDapServerProcess(launchArgs: DebugConfiguration) {
const launchArgsEnv = launchArgs.env || {};
const env = Object.assign({}, process.env, launchArgsEnv);
@@ -92,7 +87,7 @@
}
if (!fs.existsSync(dlvPath)) {
- logErrFn(
+ appendToDebugConsole(
`Couldn't find dlv at the Go tools path, ${process.env['GOPATH']}${
env['GOPATH'] ? ', ' + env['GOPATH'] : ''
} or ${envPath}`
@@ -116,13 +111,33 @@
if (launchArgs.logOutput) {
dlvArgs.push('--log-output=' + launchArgs.logOutput);
}
- logFn(`Running: ${dlvPath} ${dlvArgs.join(' ')}`);
+ appendToDebugConsole(`Running: ${dlvPath} ${dlvArgs.join(' ')}`);
const dir = parseProgramArgSync(launchArgs).dirname;
- return spawn(dlvPath, dlvArgs, {
+ const p = spawn(dlvPath, dlvArgs, {
cwd: dir,
env
});
+
+ p.stderr.on('data', (chunk) => {
+ appendToDebugConsole(chunk.toString());
+ });
+ p.stdout.on('data', (chunk) => {
+ appendToDebugConsole(chunk.toString());
+ });
+ p.on('close', (code) => {
+ if (code) {
+ appendToDebugConsole(`Process exiting with code: ${code} signal: ${p.killed}`);
+ } else {
+ appendToDebugConsole(`Process exited normally: ${p.killed}`);
+ }
+ });
+ p.on('error', (err) => {
+ if (err) {
+ appendToDebugConsole(`Error: ${err}`);
+ }
+ });
+ return p;
}
function parseProgramArgSync(
@@ -144,3 +159,8 @@
const dirname = programIsDirectory ? program : path.dirname(program);
return { program, dirname, programIsDirectory };
}
+
+function appendToDebugConsole(msg: string) {
+ // TODO(hyangah): use color distinguishable from the color used from print.
+ vscode.debug.activeDebugConsole.append(msg);
+}
To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.