Skip to content

Commit 324992b

Browse files
suzmuesuzmue
committed
src/goDebugConfiguration: allow buildFlags passed in array
For #3009 Change-Id: Icbba8db80fa6213e59152106fa01b2cf5f71ee69 Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/538057 Reviewed-by: Hyang-Ah Hana Kim <[email protected]> Commit-Queue: Suzy Mueller <[email protected]> TryBot-Result: kokoro <[email protected]>
1 parent ed3024f commit 324992b

File tree

4 files changed

+84
-33
lines changed

4 files changed

+84
-33
lines changed

‎docs/debugging.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ Here is the list of attributes specific to Go debugging.
444444
| `args` | Command line arguments passed to the debugged program.<br/>(Default: `[]`)<br/> | <center>_n/a_</center> |
445445
| `asRoot` | (Experimental) Debug with elevated permissions (on Unix). It requires `integrated` or `external` console modes and is ignored in remote debugging.<br/>(Default: `false`)<br/> | (Experimental) Debug with elevated permissions (on Unix). This requires `integrated` or `external` console modes and is ignored in remote debugging.<br/>(Default: `false`)<br/> |
446446
| `backend` | Backend used by delve. Maps to `dlv`'s `--backend` flag.<br/><p>Allowed Values: `"default"`, `"native"`, `"lldb"`, `"rr"`<br/> | <center>_same as Launch_</center>|
447-
| `buildFlags` | Build flags, to be passed to the Go compiler. Maps to dlv's `--build-flags` flag.<br/>(Default: `""`)<br/> | <center>_n/a_</center> |
447+
| `buildFlags` | Build flags, to be passed to the Go compiler. Maps to dlv's `--build-flags` flag.<br/>(Default: `[]`)<br/> | <center>_n/a_</center> |
448448
| `console` | (Experimental) Where to launch the debugger and the debug target: internal console, integrated terminal, or external terminal. It is ignored in remote debugging.<br/><p>Allowed Values: `"internalConsole"`, `"integratedTerminal"`, `"externalTerminal"`<br/>(Default: `internalConsole`)<br/> | (Experimental) Where to launch the debugger: internal console, integrated terminal, or external terminal. This does not affect tty of the running program. It is ignored in remote debugging.<br/><p>Allowed Values: `"internalConsole"`, `"integratedTerminal"`, `"externalTerminal"`<br/>(Default: `internalConsole`)<br/> |
449449
| `coreFilePath` | Path to the core dump file to open. For use on 'core' mode only<br/>(Default: `""`)<br/> | <center>_n/a_</center> |
450450
| `cwd` | Workspace relative or absolute path to the working directory of the program being debugged if a non-empty value is specified. The `program` folder is used as the working directory if `cwd` is omitted or empty.<br/>(Default: `""`)<br/> | Workspace relative or absolute path to the working directory of the program being debugged. Default is the current workspace.<br/>(Default: `"${workspaceFolder}"`)<br/> |

‎package.json

+8-2
Original file line numberDiff line numberDiff line change
@@ -711,9 +711,15 @@
711711
"default": []
712712
},
713713
"buildFlags": {
714-
"type": "string",
714+
"type": [
715+
"string",
716+
"array"
717+
],
718+
"items": {
719+
"type": "string"
720+
},
715721
"description": "Build flags, to be passed to the Go compiler. Maps to dlv's `--build-flags` flag.",
716-
"default": ""
722+
"default": []
717723
},
718724
"dlvFlags": {
719725
"type": "array",

‎src/goDebugConfiguration.ts

+41-24
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@
1010
import { lstatSync } from 'fs';
1111
import path = require('path');
1212
import vscode = require('vscode');
13+
import semver = require('semver');
1314
import { extensionId } from './const';
1415
import { getGoConfig } from './config';
1516
import { toolExecutionEnvironment } from './goEnv';
1617
import {
1718
declinedToolInstall,
19+
inspectGoToolVersion,
1820
installTools,
1921
promptForMissingTool,
2022
promptForUpdatingTool,
@@ -47,8 +49,8 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
4749
constructor(private defaultDebugAdapterType: string = 'go') {}
4850

4951
public async provideDebugConfigurations(
50-
folder: vscode.WorkspaceFolder | undefined,
51-
token?: vscode.CancellationToken
52+
_folder: vscode.WorkspaceFolder | undefined,
53+
_token?: vscode.CancellationToken
5254
): Promise<vscode.DebugConfiguration[] | undefined> {
5355
return await this.pickConfiguration();
5456
}
@@ -132,7 +134,7 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
132134
public async resolveDebugConfiguration(
133135
folder: vscode.WorkspaceFolder | undefined,
134136
debugConfiguration: vscode.DebugConfiguration,
135-
token?: vscode.CancellationToken
137+
_token?: vscode.CancellationToken
136138
): Promise<vscode.DebugConfiguration | undefined> {
137139
const activeEditor = vscode.window.activeTextEditor;
138140
if (!debugConfiguration || !debugConfiguration.request) {
@@ -261,40 +263,45 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
261263
debugConfiguration['cwd'] = resolveHomeDir(debugConfiguration['cwd']);
262264
}
263265

266+
const dlvToolPath = getBinPath('dlv');
267+
if (!path.isAbsolute(dlvToolPath)) {
268+
// If user has not already declined to install this tool,
269+
// prompt for it. Otherwise continue and have the lack of
270+
// dlv binary be caught later.
271+
if (!declinedToolInstall('dlv')) {
272+
await promptForMissingTool('dlv');
273+
return;
274+
}
275+
}
276+
debugConfiguration['dlvToolPath'] = dlvToolPath;
277+
264278
// Remove any '--gcflags' entries and show a warning
265279
if (debugConfiguration['buildFlags']) {
266-
const resp = this.removeGcflags(debugConfiguration['buildFlags']);
267-
if (resp.removed) {
268-
debugConfiguration['buildFlags'] = resp.args;
269-
this.showWarning(
270-
'ignoreDebugGCFlagsWarning',
271-
"User specified build flag '--gcflags' in 'buildFlags' is being ignored (see [debugging with build flags](https://github.com/golang/vscode-go/blob/master/docs/debugging.md#specifying-other-build-flags) documentation)"
272-
);
280+
let flags = await maybeJoinFlags(dlvToolPath, debugConfiguration['buildFlags']);
281+
if (typeof flags === 'string') {
282+
const resp = this.removeGcflags(flags);
283+
if (resp.removed) {
284+
flags = resp.args;
285+
this.showWarning(
286+
'ignoreDebugGCFlagsWarning',
287+
"User specified build flag '-gcflags' in 'buildFlags' is being ignored (see [debugging with build flags](https://github.com/golang/vscode-go/blob/master/docs/debugging.md#specifying-other-build-flags) documentation)"
288+
);
289+
}
273290
}
291+
292+
debugConfiguration['buildFlags'] = flags;
274293
}
275294
if (debugConfiguration['env'] && debugConfiguration['env']['GOFLAGS']) {
276295
const resp = this.removeGcflags(debugConfiguration['env']['GOFLAGS']);
277296
if (resp.removed) {
278297
debugConfiguration['env']['GOFLAGS'] = resp.args;
279298
this.showWarning(
280299
'ignoreDebugGCFlagsWarning',
281-
"User specified build flag '--gcflags' in 'GOFLAGS' is being ignored (see [debugging with build flags](https://github.com/golang/vscode-go/blob/master/docs/debugging.md#specifying-other-build-flags) documentation)"
300+
"User specified build flag '-gcflags' in 'GOFLAGS' is being ignored (see [debugging with build flags](https://github.com/golang/vscode-go/blob/master/docs/debugging.md#specifying-other-build-flags) documentation)"
282301
);
283302
}
284303
}
285304

286-
const dlvToolPath = getBinPath('dlv');
287-
if (!path.isAbsolute(dlvToolPath)) {
288-
// If user has not already declined to install this tool,
289-
// prompt for it. Otherwise continue and have the lack of
290-
// dlv binary be caught later.
291-
if (!declinedToolInstall('dlv')) {
292-
await promptForMissingTool('dlv');
293-
return;
294-
}
295-
}
296-
debugConfiguration['dlvToolPath'] = dlvToolPath;
297-
298305
// For dlv-dap mode, check if the dlv is recent enough to support DAP.
299306
if (debugAdapter === 'dlv-dap' && !dlvDAPVersionChecked) {
300307
const tool = getToolAtVersion('dlv');
@@ -405,7 +412,7 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
405412
public resolveDebugConfigurationWithSubstitutedVariables(
406413
folder: vscode.WorkspaceFolder | undefined,
407414
debugConfiguration: vscode.DebugConfiguration,
408-
token?: vscode.CancellationToken
415+
_token?: vscode.CancellationToken
409416
): vscode.DebugConfiguration | null {
410417
const debugAdapter = debugConfiguration['debugAdapter'];
411418
if (debugAdapter === '') {
@@ -517,6 +524,16 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
517524
}
518525
}
519526

527+
// exported for testing.
528+
export async function maybeJoinFlags(dlvToolPath: string, flags: string | string[]) {
529+
const { moduleVersion } = await inspectGoToolVersion(dlvToolPath);
530+
const localVersion = semver.parse(moduleVersion, { includePrerelease: true });
531+
if (typeof flags !== 'string' && (!localVersion || semver.lt(localVersion, '1.22.2'))) {
532+
flags = flags.join(' ');
533+
}
534+
return flags;
535+
}
536+
520537
// parseDebugProgramArgSync parses program arg of debug/auto/test launch requests.
521538
export function parseDebugProgramArgSync(
522539
program: string

‎test/integration/goDebugConfiguration.test.ts

+34-6
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import os = require('os');
77
import path = require('path');
88
import sinon = require('sinon');
99
import vscode = require('vscode');
10-
import { getGoConfig, extensionInfo } from '../../src/config';
11-
import { GoDebugConfigurationProvider } from '../../src/goDebugConfiguration';
12-
import { updateGoVarsFromConfig } from '../../src/goInstallTools';
10+
import { extensionInfo, getGoConfig } from '../../src/config';
11+
import { extensionId } from '../../src/const';
12+
import { GoDebugConfigurationProvider, maybeJoinFlags } from '../../src/goDebugConfiguration';
13+
import * as goInstallTools from '../../src/goInstallTools';
1314
import { rmdirRecursive } from '../../src/util';
14-
import goEnv = require('../../src/goEnv');
1515
import { MockCfg } from '../mocks/MockCfg';
16-
import { extensionId } from '../../src/const';
1716
import { affectedByIssue832 } from './testutils';
17+
import goEnv = require('../../src/goEnv');
1818

1919
suite('Debug Environment Variable Merge Test', () => {
2020
const debugConfigProvider = new GoDebugConfigurationProvider();
@@ -24,7 +24,7 @@ suite('Debug Environment Variable Merge Test', () => {
2424
const filePath = path.join(fixtureSourcePath, 'baseTest', 'test.go');
2525

2626
suiteSetup(async () => {
27-
await updateGoVarsFromConfig({});
27+
await goInstallTools.updateGoVarsFromConfig({});
2828
await vscode.workspace.openTextDocument(vscode.Uri.file(filePath));
2929
});
3030

@@ -507,6 +507,34 @@ suite('Debug Configuration Modify User Config', () => {
507507
assert.strictEqual(config.env.GOFLAGS, '-race -mod=mod');
508508
});
509509
});
510+
511+
suite('convert args list to string for older delve', () => {
512+
teardown(() => {
513+
sinon.restore();
514+
});
515+
516+
async function testShouldUpdateTool(
517+
input: string | string[],
518+
expected: string | string[],
519+
moduleVersion?: string
520+
) {
521+
sinon.stub(goInstallTools, 'inspectGoToolVersion').returns(Promise.resolve({ moduleVersion }));
522+
const got = await maybeJoinFlags('/path/to/dlv', input);
523+
assert.deepStrictEqual(expected, got);
524+
}
525+
526+
test('convert args list to string for older delve', async () => {
527+
await testShouldUpdateTool(['-c', 'my.conf', '-p', '8080'], '-c my.conf -p 8080', '1.5.0');
528+
});
529+
530+
test('convert args list to string for devel delve', async () => {
531+
await testShouldUpdateTool(['-c', 'my.conf', '-p', '8080'], '-c my.conf -p 8080');
532+
});
533+
534+
test('keep args list for newer delve', async () => {
535+
await testShouldUpdateTool(['-c', 'my.conf', '-p', '8080'], ['-c', 'my.conf', '-p', '8080'], '1.22.2');
536+
});
537+
});
510538
});
511539

512540
suite('Debug Configuration Resolve Paths', () => {

0 commit comments

Comments
 (0)