Skip to content

Commit 80c9d8e

Browse files
bobbrowbobbrow
committed
PR feedback
1 parent bf9bb26 commit 80c9d8e

File tree

7 files changed

+80
-55
lines changed

7 files changed

+80
-55
lines changed

‎Extension/package.json

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3377,10 +3377,10 @@
33773377
"markdownDescription": "%c_cpp.configuration.copilotHover.markdownDescription%",
33783378
"scope": "window"
33793379
},
3380-
"C_Cpp.persistDevEnvironment": {
3380+
"C_Cpp.persistVSDeveloperEnvironment": {
33813381
"type": "boolean",
33823382
"default": true,
3383-
"markdownDescription": "%c_cpp.configuration.persistDevEnvironment.markdownDescription%",
3383+
"markdownDescription": "%c_cpp.configuration.persistVSDeveloperEnvironment.markdownDescription%",
33843384
"scope": "window"
33853385
}
33863386
}
@@ -3548,13 +3548,13 @@
35483548
"icon": "$(run)"
35493549
},
35503550
{
3551-
"command": "C_Cpp.SetDevEnvironment",
3552-
"title": "%c_cpp.command.SetDevEnvironment.title%",
3551+
"command": "C_Cpp.SetVSDevEnvironment",
3552+
"title": "%c_cpp.command.SetVSDevEnvironment.title%",
35533553
"category": "C/C++"
35543554
},
35553555
{
3556-
"command": "C_Cpp.ClearDevEnvironment",
3557-
"title": "%c_cpp.command.ClearDevEnvironment.title%",
3556+
"command": "C_Cpp.ClearVSDevEnvironment",
3557+
"title": "%c_cpp.command.ClearVSDevEnvironment.title%",
35583558
"category": "C/C++"
35593559
},
35603560
{
@@ -6029,11 +6029,11 @@
60296029
"when": "editorLangId =~ /^(c|(cuda-)?cpp)$/ && config.C_Cpp.debugShortcut && cpptools.buildAndDebug.isSourceFile"
60306030
},
60316031
{
6032-
"command": "C_Cpp.SetDevEnvironment",
6032+
"command": "C_Cpp.SetVSDevEnvironment",
60336033
"when": "workspacePlatform == windows"
60346034
},
60356035
{
6036-
"command": "C_Cpp.ClearDevEnvironment",
6036+
"command": "C_Cpp.ClearVSDevEnvironment",
60376037
"when": "workspacePlatform == windows"
60386038
},
60396039
{

‎Extension/package.nls.json

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@
3737
"c_cpp.command.RemoveAllCodeAnalysisProblems.title": "Clear All Code Analysis Problems",
3838
"c_cpp.command.BuildAndDebugFile.title": "Debug C/C++ File",
3939
"c_cpp.command.BuildAndRunFile.title": "Run C/C++ File",
40-
"c_cpp.command.SetDevEnvironment.title": "Set Developer Environment",
41-
"c_cpp.command.ClearDevEnvironment.title": "Clear Developer Environment",
40+
"c_cpp.command.SetVSDevEnvironment.title": "Set Visual Studio Developer Environment",
41+
"c_cpp.command.ClearVSDevEnvironment.title": "Clear Visual Studio Developer Environment",
4242
"c_cpp.command.AddDebugConfiguration.title": "Add Debug Configuration",
4343
"c_cpp.command.GenerateDoxygenComment.title": "Generate Doxygen Comment",
4444
"c_cpp.command.addSshTarget.title": "Add SSH target",
@@ -845,7 +845,7 @@
845845
]
846846
},
847847
"c_cpp.configuration.debugShortcut.description": "Show the \"Run and Debug\" play button and \"Add Debug Configuration\" gear in the editor title bar for C++ files.",
848-
"c_cpp.configuration.persistDevEnvironment.markdownDescription": "Remember the last used Visual Studio developer environment for the current workspace. This setting is only applicable for Windows.",
848+
"c_cpp.configuration.persistVSDeveloperEnvironment.markdownDescription": "Remember the last used Visual Studio developer environment for the current workspace. This setting is only applicable for Windows.",
849849
"c_cpp.debuggers.pipeTransport.description": "When present, this tells the debugger to connect to a remote computer using another executable as a pipe that will relay standard input/output between VS Code and the MI-enabled debugger backend executable (such as gdb).",
850850
"c_cpp.debuggers.pipeTransport.default.pipeProgram": "enter the fully qualified path for the pipe program name, for example '/usr/bin/ssh'.",
851851
"c_cpp.debuggers.pipeTransport.default.debuggerPath": "The full path to the debugger on the target machine, for example /usr/bin/gdb.",
@@ -1034,9 +1034,9 @@
10341034
]
10351035
},
10361036
"c_cpp.walkthrough.command.prompt.description": {
1037-
"message": "When using the Microsoft Visual Studio C++ compiler, the Visual Studio Developer Environment must be present.\n\nFollow the instructions on the right to relaunch or click the button below.\n[Set Developer Environment](command:C_Cpp.SetDevEnvironment?%22walkthrough%22)",
1037+
"message": "When using the Microsoft Visual Studio C++ compiler, the Visual Studio Developer Environment must be present.\n\nFollow the instructions on the right to relaunch or click the button below.\n[Set Developer Environment](command:C_Cpp.SetVSDevEnvironment?%22walkthrough%22)",
10381038
"comment": [
1039-
"{Locked=\"Visual Studio\"} {Locked=\"C++\"} {Locked=\"\\\n[\"} {Locked=\"](C_Cpp.SetDevEnvironment?%22walkthrough%22)\"}"
1039+
"{Locked=\"Visual Studio\"} {Locked=\"C++\"} {Locked=\"\\\n[\"} {Locked=\"](C_Cpp.SetVSDevEnvironment?%22walkthrough%22)\"}"
10401040
]
10411041
},
10421042
"c_cpp.walkthrough.run.debug.title": "Run and debug your C++ file",

‎Extension/src/Debugger/configurationProvider.ts

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -586,31 +586,37 @@ export class DebugConfigurationProvider implements vscode.DebugConfigurationProv
586586
* @returns `true` if the Developer Environment is not available and an error was shown to the user, `false` if the Developer Environment is available or the user chose to apply it.
587587
*/
588588
private async showErrorIfClNotAvailable(_configurationLabel: string): Promise<boolean> {
589-
if (!util.hasMsvcEnvironment()) {
590-
const applyDevEnv = localize("apply.dev.env", "Apply Developer Environment");
591-
const cancel = localize("cancel", "Cancel");
592-
const response = await vscode.window.showErrorMessage(
593-
localize({
594-
key: "cl.exe.not.available",
595-
comment: ["{0} is a command option in a menu."]
596-
}, "{0} requires the Visual Studio Developer Environment.", `cl.exe ${this.buildAndDebugActiveFileStr()}`),
597-
applyDevEnv,
598-
cancel);
599-
if (response === applyDevEnv) {
600-
try {
601-
await vscode.commands.executeCommand('C_Cpp.SetDevEnvironment', 'buildAndDebug');
602-
} catch {
603-
// Ignore the error, the user will be prompted to apply the environment manually.
604-
}
605-
}
606-
if (util.hasMsvcEnvironment()) {
607-
return false;
589+
if (util.hasMsvcEnvironment()) {
590+
return false; // No error to show
591+
}
592+
593+
const applyDevEnv = localize("apply.dev.env", "Apply Developer Environment");
594+
const cancel = localize("cancel", "Cancel");
595+
const response = await vscode.window.showErrorMessage(
596+
localize({
597+
key: "cl.exe.not.available",
598+
comment: ["{0} is a command option in a menu."]
599+
}, "{0} requires the Visual Studio Developer Environment.", `cl.exe ${this.buildAndDebugActiveFileStr()}`),
600+
applyDevEnv,
601+
cancel);
602+
if (response === applyDevEnv) {
603+
try {
604+
await vscode.commands.executeCommand('C_Cpp.SetVSDevEnvironment', 'buildAndDebug');
605+
} catch (e: any) {
606+
// Ignore the error, the user will be prompted to apply the environment manually.
608607
}
609-
void vscode.window.showErrorMessage(
610-
localize('dev.env.not.applied', 'The Visual Studio Developer Environment was not applied. Please try again or run VS Code from the Developer Command Prompt for VS.'));
608+
}
609+
if (response === cancel) {
610+
// A message was already shown, so exit early noting that the environment is not available. We don't need to show another error message.
611611
return true;
612612
}
613-
return false;
613+
614+
if (util.hasMsvcEnvironment()) {
615+
return false;
616+
}
617+
void vscode.window.showErrorMessage(
618+
localize('dev.env.not.applied', 'The Visual Studio Developer Environment was not applied. Please try again or run VS Code from the Developer Command Prompt for VS.'));
619+
return true;
614620
}
615621

616622
private getLLDBFrameworkPath(): string | undefined {

‎Extension/src/LanguageServer/devcmd.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFo
1616
const localize: nls.LocalizeFunc = nls.loadMessageBundle();
1717

1818
const errorNoContext = localize('no.context.provided', 'No context provided');
19-
const errorNotWindows = localize('not.windows', 'This command is only available on Windows');
19+
const errorNotWindows = localize('not.windows', 'The "Set Visual Studio Developer Environment" command is only available on Windows');
2020
const errorNoVSFound = localize('error.no.vs', 'A Visual Studio installation with the C++ compiler was not found');
21-
const errorOperationCancelled = localize('operation.cancelled', 'The operation was cancelled');
21+
export const errorOperationCancelled = localize('operation.cancelled', 'The operation was cancelled');
2222
const errorNoHostsFound = localize('no.hosts', 'No hosts found');
2323
const configuringDevEnv = localize('config.dev.env', 'Configuring Developer Environment...');
2424
const selectVSInstallation = localize('select.vs.install', 'Select a Visual Studio installation');
@@ -74,7 +74,7 @@ export async function setEnvironment(context?: vscode.ExtensionContext) {
7474
for (const key of Object.keys(vars)) {
7575
context.environmentVariableCollection.replace(key, vars[key].replace(`%${key}%`, '${env:' + key + '}'));
7676
}
77-
context.environmentVariableCollection.description = localize('dev.env.for', '{0} Developer Environment for {1}', arch, vs.displayName);
77+
context.environmentVariableCollection.description = localize('dev.env.for', '{0} Developer Environment for {1}', arch, vsDisplayNameWithSuffix(vs));
7878
context.environmentVariableCollection.persistent = settings.persistDevEnvironment;
7979

8080
return true;
@@ -94,10 +94,24 @@ async function getVSInstallations() {
9494
return installations;
9595
}
9696

97+
function vsDisplayNameWithSuffix(installation: vswhere.Installation): string {
98+
const suffix = (() => {
99+
if (installation.channelId.endsWith('.main')) {
100+
return 'main';
101+
} else if (installation.channelId.endsWith('.IntPreview')) {
102+
return 'Int Preview';
103+
} else if (installation.channelId.endsWith('.Preview')) {
104+
return 'Preview';
105+
}
106+
return '';
107+
})();
108+
return `${installation.displayName}${suffix ? ` ${suffix}` : ''}`;
109+
}
110+
97111
async function chooseVSInstallation(installations: vswhere.Installation[]): Promise<vswhere.Installation | undefined> {
98112
const items: vscode.QuickPickItem[] = installations.map(installation => <vscode.QuickPickItem>{
99-
label: installation.displayName,
100-
description: localize('default.settings', 'Default settings for {0}', installation.displayName)
113+
label: vsDisplayNameWithSuffix(installation),
114+
detail: localize('default.env', 'Default environment for {0}', installation.catalog.productDisplayVersion)
101115
});
102116
items.push({
103117
label: advancedOptions,
@@ -110,7 +124,7 @@ async function chooseVSInstallation(installations: vswhere.Installation[]): Prom
110124
throw new Error(errorOperationCancelled);
111125
}
112126

113-
return installations.find(installation => installation.displayName === selection.label);
127+
return installations.find(installation => vsDisplayNameWithSuffix(installation) === selection.label);
114128
}
115129

116130
interface Compiler {
@@ -143,15 +157,15 @@ async function chooseCompiler(vses: vswhere.Installation[]): Promise<Compiler |
143157
}
144158
const items = compilers.map(compiler => <vscode.QuickPickItem>{
145159
label: compiler.version,
146-
description: compiler.vs.displayName
160+
description: vsDisplayNameWithSuffix(compiler.vs)
147161
});
148162
const selection = await vscode.window.showQuickPick(items, {
149163
placeHolder: selectToolsetVersion
150164
});
151165
if (!selection) {
152166
throw new Error(errorOperationCancelled);
153167
}
154-
return compilers.find(compiler => compiler.version === selection.label && compiler.vs.displayName === selection.description);
168+
return compilers.find(compiler => compiler.version === selection.label && vsDisplayNameWithSuffix(compiler.vs) === selection.description);
155169
}
156170

157171
async function setOptions(compiler: Compiler): Promise<void> {

‎Extension/src/LanguageServer/extension.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { CodeActionDiagnosticInfo, CodeAnalysisDiagnosticIdentifiersAndUri, code
2929
import { registerRelatedFilesProvider } from './copilotProviders';
3030
import { CppBuildTaskProvider } from './cppBuildTaskProvider';
3131
import { getCustomConfigProviders } from './customProviders';
32-
import { setEnvironment } from './devcmd';
32+
import { errorOperationCancelled, setEnvironment } from './devcmd';
3333
import { getLanguageConfig } from './languageConfig';
3434
import { CppConfigurationLanguageModelTool } from './lmTool';
3535
import { getLocaleId } from './localization';
@@ -431,8 +431,8 @@ export async function registerCommands(enabled: boolean): Promise<void> {
431431
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.ExtractToMemberFunction', enabled ? () => onExtractToFunction(false, true) : onDisabledCommand));
432432
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.ExpandSelection', enabled ? (r: Range) => onExpandSelection(r) : onDisabledCommand));
433433
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.ShowCopilotHover', enabled ? () => onCopilotHover() : onDisabledCommand));
434-
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.SetDevEnvironment', enabled ? onSetDevEnvironment : onDisabledCommand));
435-
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.ClearDevEnvironment', enabled ? onClearDevEnvironment : onDisabledCommand));
434+
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.SetVSDevEnvironment', enabled ? onSetVSDevEnvironment : onDisabledCommand));
435+
commandDisposables.push(vscode.commands.registerCommand('C_Cpp.ClearVSDevEnvironment', enabled ? onClearVSDevEnvironment : onDisabledCommand));
436436
}
437437

438438
function onDisabledCommand() {
@@ -1562,23 +1562,27 @@ async function showCopilotContent(copilotHoverProvider: CopilotHoverProvider, ho
15621562
return true;
15631563
}
15641564

1565-
async function onSetDevEnvironment(sender?: any): Promise<void> {
1565+
async function onSetVSDevEnvironment(sender?: any): Promise<void> {
15661566
let success: boolean = true;
15671567
try {
15681568
await setEnvironment(util.extensionContext);
15691569
await vscode.commands.executeCommand('setContext', 'cpptools.msvcEnvironmentFound', util.hasMsvcEnvironment());
1570-
void vscode.window.showInformationMessage(`${util.extensionContext?.environmentVariableCollection.description} successfully set.`);
1570+
if (sender !== 'buildAndDebug') {
1571+
void vscode.window.showInformationMessage(`${util.extensionContext?.environmentVariableCollection.description} successfully set.`);
1572+
}
15711573
} catch (error: any) {
15721574
success = false;
15731575
if (!isWindows) {
15741576
throw error;
15751577
}
1576-
void vscode.window.showErrorMessage(`Developer environment not set: ${error.message}`);
1578+
if (error.message !== errorOperationCancelled) {
1579+
void vscode.window.showErrorMessage(`Failed to apply VS developer environment: ${error.message}`);
1580+
}
15771581
}
1578-
telemetry.logLanguageServerEvent("SetDevEnvironment", { "sender": util.getSenderType(sender), success: success.toString() });
1582+
telemetry.logLanguageServerEvent("SetVSDevEnvironment", { "sender": util.getSenderType(sender), success: success.toString() });
15791583
}
15801584

1581-
async function onClearDevEnvironment(): Promise<void> {
1585+
async function onClearVSDevEnvironment(): Promise<void> {
15821586
util.extensionContext?.environmentVariableCollection.clear();
15831587
await vscode.commands.executeCommand('setContext', 'cpptools.msvcEnvironmentFound', util.hasMsvcEnvironment());
15841588
}

‎Extension/src/LanguageServer/settings.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ export class CppSettings extends Settings {
549549
&& this.intelliSenseEngine.toLowerCase() === "default" && vscode.workspace.getConfiguration("workbench").get<any>("colorTheme") !== "Default High Contrast";
550550
}
551551
public get sshTargetsView(): string { return this.getAsString("sshTargetsView"); }
552-
public get persistDevEnvironment(): boolean { return this.getAsBoolean("persistDevEnvironment"); }
552+
public get persistDevEnvironment(): boolean { return this.getAsBoolean("persistVSDeveloperEnvironment"); }
553553

554554
// Returns the value of a setting as a string with proper type validation and checks for valid enum values while returning an undefined value if necessary.
555555
private getAsStringOrUndefined(settingName: string): string | undefined {

‎Extension/test/scenarios/SimpleCppProject/tests/devEnvironment.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ import { suite } from 'mocha';
77
import * as vscode from 'vscode';
88
import * as util from '../../../../src/common';
99
import { isWindows } from "../../../../src/constants";
10+
import { errorOperationCancelled } from '../../../../src/LanguageServer/devcmd';
1011

1112
suite("set developer environment", () => {
1213
if (isWindows) {
1314
test("set developer environment (Windows)", async () => {
14-
const promise = vscode.commands.executeCommand('C_Cpp.SetDevEnvironment', 'test');
15+
const promise = vscode.commands.executeCommand('C_Cpp.SetVSDevEnvironment', 'test');
1516
const timer = setInterval(() => {
1617
void vscode.commands.executeCommand('workbench.action.acceptSelectedQuickOpenItem');
1718
}, 1000);
@@ -22,11 +23,11 @@ suite("set developer environment", () => {
2223
} else {
2324
test("set developer environment (Linux/macOS)", async () => {
2425
try {
25-
await vscode.commands.executeCommand('C_Cpp.SetDevEnvironment', 'test');
26+
await vscode.commands.executeCommand('C_Cpp.SetVSDevEnvironment', 'test');
2627
equal(false, true, "Should not be able to set developer environment on non-Windows platform.");
2728
}
2829
catch (e) {
29-
equal((e as Error).message, 'This command is only available on Windows', "Should throw error when trying to set developer environment on non-Windows platform.");
30+
equal((e as Error).message, errorOperationCancelled, "Should throw error when trying to set developer environment on non-Windows platform.");
3031
}
3132
equal(util.hasMsvcEnvironment(), false, "MSVC environment should not be set on non-Windows platforms.");
3233
});

0 commit comments

Comments
 (0)