[vscode-go] src/goDebugFactory: redirect stdout/stderr from dlv to Debug Console

226 views
Skip to first unread message

Suzy Mueller (Gerrit)

unread,
Mar 4, 2021, 12:50:51 PM3/4/21
to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, kokoro, Polina Sokolova, golang-co...@googlegroups.com

Attention is currently required from: Hyang-Ah Hana Kim.

Patch set 1:Code-Review +2

View Change

    To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: vscode-go
    Gerrit-Branch: master
    Gerrit-Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
    Gerrit-Change-Number: 296930
    Gerrit-PatchSet: 1
    Gerrit-Owner: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-CC: Polina Sokolova <pol...@google.com>
    Gerrit-Attention: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Comment-Date: Thu, 04 Mar 2021 17:50:46 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Polina Sokolova (Gerrit)

    unread,
    Mar 4, 2021, 1:21:45 PM3/4/21
    to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, kokoro, golang-co...@googlegroups.com

    Attention is currently required from: Hyang-Ah Hana Kim.

    View Change

    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.

    Gerrit-Project: vscode-go
    Gerrit-Branch: master
    Gerrit-Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
    Gerrit-Change-Number: 296930
    Gerrit-PatchSet: 1
    Gerrit-Owner: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-CC: Polina Sokolova <pol...@google.com>
    Gerrit-Attention: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Comment-Date: Thu, 04 Mar 2021 18:21:40 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Suzy Mueller (Gerrit)

    unread,
    Mar 4, 2021, 2:54:46 PM3/4/21
    to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, kokoro, Polina Sokolova, golang-co...@googlegroups.com

    Attention is currently required from: Hyang-Ah Hana Kim, Polina Sokolova.

    View Change

    1 comment:

    • File src/goDebugFactory.ts:

      • 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.

    Gerrit-Project: vscode-go
    Gerrit-Branch: master
    Gerrit-Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
    Gerrit-Change-Number: 296930
    Gerrit-PatchSet: 1
    Gerrit-Owner: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-CC: Polina Sokolova <pol...@google.com>
    Gerrit-Attention: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Attention: Polina Sokolova <pol...@google.com>
    Gerrit-Comment-Date: Thu, 04 Mar 2021 19:54:41 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Polina Sokolova <pol...@google.com>
    Gerrit-MessageType: comment

    Polina Sokolova (Gerrit)

    unread,
    Mar 4, 2021, 10:29:42 PM3/4/21
    to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, kokoro, golang-co...@googlegroups.com

    Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.

    View Change

    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

    To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: vscode-go
    Gerrit-Branch: master
    Gerrit-Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
    Gerrit-Change-Number: 296930
    Gerrit-PatchSet: 1
    Gerrit-Owner: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-CC: Polina Sokolova <pol...@google.com>
    Gerrit-Attention: Suzy Mueller <suz...@golang.org>
    Gerrit-Attention: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Comment-Date: Fri, 05 Mar 2021 03:29:38 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Suzy Mueller <suz...@golang.org>

    Hyang-Ah Hana Kim (Gerrit)

    unread,
    Mar 5, 2021, 12:18:32 AM3/5/21
    to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, kokoro, Polina Sokolova, golang-co...@googlegroups.com

    Attention is currently required from: Suzy Mueller, Polina Sokolova.

    View Change

    2 comments:

    • File src/goDebugFactory.ts:

      • 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.

      • 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.

    Gerrit-Project: vscode-go
    Gerrit-Branch: master
    Gerrit-Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
    Gerrit-Change-Number: 296930
    Gerrit-PatchSet: 1
    Gerrit-Owner: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-CC: Polina Sokolova <pol...@google.com>
    Gerrit-Attention: Suzy Mueller <suz...@golang.org>
    Gerrit-Attention: Polina Sokolova <pol...@google.com>
    Gerrit-Comment-Date: Fri, 05 Mar 2021 05:18:26 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No

    Polina Sokolova (Gerrit)

    unread,
    Mar 5, 2021, 2:27:33 AM3/5/21
    to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Suzy Mueller, kokoro, golang-co...@googlegroups.com

    Attention is currently required from: Suzy Mueller, Hyang-Ah Hana Kim.

    Patch set 1:Code-Review +2

    View Change

    3 comments:

    • Patchset:

      • Patch Set #1:

        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:

      • 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".

      • 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.

    Gerrit-Project: vscode-go
    Gerrit-Branch: master
    Gerrit-Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
    Gerrit-Change-Number: 296930
    Gerrit-PatchSet: 1
    Gerrit-Owner: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Polina Sokolova <pol...@google.com>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-Attention: Suzy Mueller <suz...@golang.org>
    Gerrit-Attention: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Comment-Date: Fri, 05 Mar 2021 07:27:28 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Hyang-Ah Hana Kim <hya...@gmail.com>

    Hyang-Ah Hana Kim (Gerrit)

    unread,
    Mar 9, 2021, 12:23:02 AM3/9/21
    to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Polina Sokolova, Suzy Mueller, kokoro, golang-co...@googlegroups.com

    Attention is currently required from: Suzy Mueller.

    View Change

    2 comments:

    • File src/goDebugFactory.ts:

      • Ah, I see. I thought it was printing to Debug Console and was controlled by "trace".

        Ack

      • Done

    To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: vscode-go
    Gerrit-Branch: master
    Gerrit-Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
    Gerrit-Change-Number: 296930
    Gerrit-PatchSet: 1
    Gerrit-Owner: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Polina Sokolova <pol...@google.com>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-Attention: Suzy Mueller <suz...@golang.org>
    Gerrit-Comment-Date: Tue, 09 Mar 2021 05:22:58 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No

    Hyang-Ah Hana Kim (Gerrit)

    unread,
    Mar 9, 2021, 12:40:29 AM3/9/21
    to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

    Attention is currently required from: Suzy Mueller.

    Hyang-Ah Hana Kim uploaded patch set #2 to this change.

    View 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.

    Gerrit-Project: vscode-go
    Gerrit-Branch: master
    Gerrit-Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
    Gerrit-Change-Number: 296930
    Gerrit-PatchSet: 2
    Gerrit-Owner: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Polina Sokolova <pol...@google.com>
    Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
    Gerrit-Reviewer: kokoro <noreply...@google.com>
    Gerrit-Attention: Suzy Mueller <suz...@golang.org>
    Gerrit-MessageType: newpatchset

    kokoro (Gerrit)

    unread,
    Mar 9, 2021, 12:52:39 AM3/9/21
    to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, Polina Sokolova, Suzy Mueller, golang-co...@googlegroups.com

    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

    View Change

      To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: vscode-go
      Gerrit-Branch: master
      Gerrit-Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
      Gerrit-Change-Number: 296930
      Gerrit-PatchSet: 2
      Gerrit-Owner: Hyang-Ah Hana Kim <hya...@gmail.com>
      Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
      Gerrit-Reviewer: Polina Sokolova <pol...@google.com>
      Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
      Gerrit-Reviewer: kokoro <noreply...@google.com>
      Gerrit-Attention: Suzy Mueller <suz...@golang.org>
      Gerrit-Comment-Date: Tue, 09 Mar 2021 05:52:35 +0000

      Hyang-Ah Hana Kim (Gerrit)

      unread,
      Mar 9, 2021, 1:26:50 AM3/9/21
      to Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, kokoro, Polina Sokolova, Suzy Mueller, golang-co...@googlegroups.com

      Hyang-Ah Hana Kim submitted this change.

      View Change

      Approvals: Suzy Mueller: Looks good to me, approved Polina Sokolova: Looks good to me, approved Hyang-Ah Hana Kim: Trusted; Run TryBots kokoro: TryBots succeeded
      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);
      +}

      1 is the latest approved patch-set. The change was submitted with unreviewed changes in the following files: The name of the file: src/goDebugFactory.ts Insertions: 8, Deletions: 12. ``` @@ -89:90, +89:90 @@ - printError( + appendToDebugConsole( @@ -113:114, +113:114 @@ - print(`Running: ${dlvPath} ${dlvArgs.join(' ')}`); + appendToDebugConsole(`Running: ${dlvPath} ${dlvArgs.join(' ')}`); @@ -122:123, +122:123 @@ - printError(chunk.toString()); + appendToDebugConsole(chunk.toString()); @@ -125:126, +125:126 @@ - print(chunk.toString()); + appendToDebugConsole(chunk.toString()); @@ -129:132, +129:132 @@ - printError(`Process exiting with code: ${code} signal: ${p.killed}`); - } else { - print(`Process exited normally: ${p.killed}`); + appendToDebugConsole(`Process exiting with code: ${code} signal: ${p.killed}`); + } else { + appendToDebugConsole(`Process exited normally: ${p.killed}`); @@ -136:137, +136:137 @@ - printError(`Error: ${err}`); + appendToDebugConsole(`Error: ${err}`); @@ -155:156, +155:156 @@ - if (!programIsDirectory && (path.extname(program) !== '.go' || launchArgs.mode === 'exec')) { + if (!programIsDirectory && launchArgs.mode !== 'exec' && path.extname(program) !== '.go') { @@ -162:163, +162:163 @@ - function printError(msg: string) { + function appendToDebugConsole(msg: string) { @@ -166:170 @@ - - function print(msg: string) { - vscode.debug.activeDebugConsole.append(msg); - } ```

      To view, visit change 296930. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: vscode-go
      Gerrit-Branch: master
      Gerrit-Change-Id: I66c62a14ab5659e0789f2d9b16274048995640c4
      Gerrit-Change-Number: 296930
      Gerrit-PatchSet: 3
      Gerrit-Owner: Hyang-Ah Hana Kim <hya...@gmail.com>
      Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
      Gerrit-Reviewer: Polina Sokolova <pol...@google.com>
      Gerrit-Reviewer: Suzy Mueller <suz...@golang.org>
      Gerrit-Reviewer: kokoro <noreply...@google.com>
      Gerrit-MessageType: merged
      Reply all
      Reply to author
      Forward
      0 new messages