Skip to content

Commit

Permalink
src/goDebugFactory,src/goDebug: only add --log-output flag if --log i…
Browse files Browse the repository at this point in the history
…s set

In order to enable logging, debug users can set "showLog": true and
specify the output with "logOutput". These map directly to the delve
flags "--log" and "--log-output". If "--log-output" is passed without
"--log" delve complains. This means that users always have to remove
"--log-output" if they want to disable logging. This change only
passes the "--log-output" to delve if "showLog": true. This allows
users to disable logging by simply setting "showLog": false.

Change-Id: Ic568fbebe6b5ab07b2b5b2e60b6c68659cd67430
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/335029
Trust: Suzy Mueller <[email protected]>
Run-TryBot: Suzy Mueller <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
  • Loading branch information
suzmue committed Jul 23, 2021
1 parent 93a6e13 commit 15aaa8c
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 11 deletions.
8 changes: 5 additions & 3 deletions src/debugAdapter/goDebug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -639,9 +639,11 @@ export class Delve {

if (launchArgs.showLog) {
dlvArgs.push('--log=' + launchArgs.showLog.toString());
}
if (launchArgs.logOutput) {
dlvArgs.push('--log-output=' + launchArgs.logOutput);
// Only add the log output flag if we have already added the log flag.
// Otherwise, delve complains.
if (launchArgs.logOutput) {
dlvArgs.push('--log-output=' + launchArgs.logOutput);
}
}
if (launchArgs.cwd) {
dlvArgs.push('--wd=' + launchArgs.cwd);
Expand Down
8 changes: 5 additions & 3 deletions src/goDebugFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,11 @@ function spawnDlvDapServerProcess(
dlvArgs.push(`--listen=${host}:${port}`);
if (launchAttachArgs.showLog) {
dlvArgs.push('--log=' + launchAttachArgs.showLog.toString());
}
if (launchAttachArgs.logOutput) {
dlvArgs.push('--log-output=' + launchAttachArgs.logOutput);
// Only add the log output flag if we have already added the log flag.
// Otherwise, delve complains.
if (launchAttachArgs.logOutput) {
dlvArgs.push('--log-output=' + launchAttachArgs.logOutput);
}
}

const onWindows = process.platform === 'win32';
Expand Down
28 changes: 23 additions & 5 deletions test/integration/goDebug.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,22 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean) => {
throw new Error('does not report error on invalid delve flag');
});

test('should run program with showLog=false and logOutput specified', async () => {
const PROGRAM = path.join(DATA_ROOT, 'baseTest');

const config = {
name: 'Launch',
type: 'go',
request: 'launch',
mode: 'debug',
program: PROGRAM,
showLog: false,
logOutput: 'dap'
};
const debugConfig = await initializeDebugConfig(config, true);
await Promise.all([dc.configurationSequence(), dc.launch(debugConfig), dc.waitForEvent('terminated')]);
});

test('should handle threads request after initialization', async () => {
const PROGRAM = path.join(DATA_ROOT, 'baseTest');

Expand Down Expand Up @@ -2068,13 +2084,15 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean) => {
});

let testNumber = 0;
async function initializeDebugConfig(config: DebugConfiguration) {
async function initializeDebugConfig(config: DebugConfiguration, keepUserLog?: boolean) {
if (isDlvDap) {
config['debugAdapter'] = 'dlv-dap';
// Log the output for easier test debugging.
config['logOutput'] = 'dap,debugger';
config['showLog'] = true;
config['trace'] = 'verbose';
if (!keepUserLog) {
// Log the output for easier test debugging.
config['logOutput'] = 'dap,debugger';
config['showLog'] = true;
config['trace'] = 'verbose';
}
} else {
config['debugAdapter'] = 'legacy';
// be explicit and prevent resolveDebugConfiguration from picking
Expand Down

0 comments on commit 15aaa8c

Please sign in to comment.