Skip to content

Commit 3b9e2f3

Browse files
spboyerspboyer
committed
Address PR feedback for VS Code extension views
- Rename WorkspaceViewExtensionResolve to ExtensionsViewResolve (correct view ID) - Remove fragile caching test from azureDevTemplateProvider.test.ts - Add copyright headers to view providers - Add vscode.l10n.t() for user-visible strings in HelpAndFeedbackTreeDataProvider - Rename toggle commands to toggleEnvVarVisibility for clarity - Make Help and Feedback view collapsed by default - Use specific context values for templates to avoid conflicts - Consolidate duplicate environment menu entries using regex alternation - Add NLS strings to package.nls.json for new commands - Fix refresh command registration (define command, remove @1) - Restore telemetryId parameter in registerActivityCommand - Remove redundant azureWorkspace.refresh from refreshEnvironment - Add comments explaining hard-coded delays in RevealStep.ts
1 parent 243e468 commit 3b9e2f3

File tree

15 files changed

+85
-96
lines changed

15 files changed

+85
-96
lines changed

‎ext/vscode/package.json‎

Lines changed: 30 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -130,23 +130,23 @@
130130
{
131131
"category": "%azure-dev.commands_category%",
132132
"command": "azure-dev.commands.cli.extension-install",
133-
"title": "Install Extension",
133+
"title": "%azure-dev.commands.cli.extension-install.title%",
134134
"icon": "$(add)"
135135
},
136136
{
137137
"category": "%azure-dev.commands_category%",
138138
"command": "azure-dev.commands.cli.extension-uninstall",
139-
"title": "Uninstall Extension"
139+
"title": "%azure-dev.commands.cli.extension-uninstall.title%"
140140
},
141141
{
142142
"category": "%azure-dev.commands_category%",
143143
"command": "azure-dev.commands.cli.extension-upgrade",
144-
"title": "Upgrade Extension"
144+
"title": "%azure-dev.commands.cli.extension-upgrade.title%"
145145
},
146146
{
147147
"category": "%azure-dev.commands_category%",
148148
"command": "azure-dev.commands.addService",
149-
"title": "Add Service...",
149+
"title": "%azure-dev.commands.addService.title%",
150150
"icon": "$(add)"
151151
},
152152
{
@@ -181,37 +181,43 @@
181181
},
182182
{
183183
"category": "%azure-dev.commands_category%",
184-
"command": "azure-dev.views.environments.toggleVisibility",
185-
"title": "Toggle Visibility"
184+
"command": "azure-dev.views.environments.toggleEnvVarVisibility",
185+
"title": "%azure-dev.views.environments.toggleEnvVarVisibility.title%"
186186
},
187187
{
188188
"category": "%azure-dev.commands_category%",
189-
"command": "azure-dev.commands.workspace.toggleVisibility",
190-
"title": "Toggle Visibility"
189+
"command": "azure-dev.views.workspace.toggleEnvVarVisibility",
190+
"title": "%azure-dev.views.workspace.toggleEnvVarVisibility.title%"
191191
},
192192
{
193193
"category": "%azure-dev.commands_category%",
194194
"command": "azure-dev.views.environments.viewDotEnv",
195-
"title": "View .env file",
195+
"title": "%azure-dev.views.environments.viewDotEnv.title%",
196196
"icon": "$(go-to-file)"
197197
},
198198
{
199199
"category": "%azure-dev.commands_category%",
200200
"command": "azure-dev.views.templateTools.openReadme",
201-
"title": "View README",
201+
"title": "%azure-dev.views.templateTools.openReadme.title%",
202202
"icon": "$(book)"
203203
},
204204
{
205205
"category": "%azure-dev.commands_category%",
206206
"command": "azure-dev.views.templateTools.openGitHub",
207-
"title": "View on GitHub",
207+
"title": "%azure-dev.views.templateTools.openGitHub.title%",
208208
"icon": "$(github)"
209209
},
210210
{
211211
"category": "%azure-dev.commands_category%",
212212
"command": "azure-dev.views.templateTools.initFromTemplateInline",
213-
"title": "Initialize from Template",
213+
"title": "%azure-dev.views.templateTools.initFromTemplateInline.title%",
214214
"icon": "$(rocket)"
215+
},
216+
{
217+
"category": "%azure-dev.commands_category%",
218+
"command": "azure-dev.views.templateTools.refresh",
219+
"title": "%azure-dev.views.templateTools.refresh.title%",
220+
"icon": "$(refresh)"
215221
}
216222
],
217223
"viewsContainers": {
@@ -243,7 +249,8 @@
243249
},
244250
{
245251
"id": "azure-dev.views.helpAndFeedback",
246-
"name": "Help and Feedback"
252+
"name": "Help and Feedback",
253+
"visibility": "collapsed"
247254
}
248255
]
249256
},
@@ -277,8 +284,7 @@
277284
{
278285
"command": "azure-dev.views.templateTools.refresh",
279286
"when": "view == azure-dev.views.templateTools",
280-
"group": "navigation@1",
281-
"icon": "$(refresh)"
287+
"group": "navigation"
282288
}
283289
],
284290
"commandPalette": [
@@ -384,22 +390,22 @@
384390
"view/item/context": [
385391
{
386392
"command": "azure-dev.views.templateTools.initFromTemplateInline",
387-
"when": "viewItem == template",
393+
"when": "viewItem == ms-azuretools.azure-dev.views.templateTools.template",
388394
"group": "inline@10"
389395
},
390396
{
391397
"command": "azure-dev.views.templateTools.openGitHub",
392-
"when": "viewItem == template",
398+
"when": "viewItem == ms-azuretools.azure-dev.views.templateTools.template",
393399
"group": "inline@20"
394400
},
395401
{
396402
"command": "azure-dev.views.templateTools.openReadme",
397-
"when": "viewItem == template",
403+
"when": "viewItem == ms-azuretools.azure-dev.views.templateTools.template",
398404
"group": "10template@10"
399405
},
400406
{
401407
"command": "azure-dev.views.templateTools.openGitHub",
402-
"when": "viewItem == template",
408+
"when": "viewItem == ms-azuretools.azure-dev.views.templateTools.template",
403409
"group": "10template@20"
404410
},
405411
{
@@ -454,44 +460,24 @@
454460
},
455461
{
456462
"command": "azure-dev.commands.cli.env-select",
457-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.workspace.environment(?!s)(?!.*default)/i",
463+
"when": "viewItem =~ /ms-azuretools.azure-dev.views.(workspace.environment(?!s)|environments.environment)(?!.*default)/i",
458464
"group": "20env@20"
459465
},
460-
{
461-
"command": "azure-dev.commands.cli.env-select",
462-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.environments.environment(?!.*default)/i",
463-
"group": "10env@10"
464-
},
465466
{
466467
"command": "azure-dev.commands.cli.env-refresh",
467-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.workspace.environment(?!s)/i",
468+
"when": "viewItem =~ /ms-azuretools.azure-dev.views.(workspace.environment(?!s)|environments.environment)/i",
468469
"group": "20env@30"
469470
},
470-
{
471-
"command": "azure-dev.commands.cli.env-refresh",
472-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.environments.environment/i",
473-
"group": "10env@20"
474-
},
475471
{
476472
"command": "azure-dev.commands.cli.env-edit",
477-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.workspace.environment(?!s)/i",
473+
"when": "viewItem =~ /ms-azuretools.azure-dev.views.(workspace.environment(?!s)|environments.environment)/i",
478474
"group": "20env@40"
479475
},
480-
{
481-
"command": "azure-dev.commands.cli.env-edit",
482-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.environments.environment/i",
483-
"group": "10env@30"
484-
},
485476
{
486477
"command": "azure-dev.commands.cli.env-delete",
487-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.workspace.environment(?!s)(?!.*default)/i",
478+
"when": "viewItem =~ /ms-azuretools.azure-dev.views.(workspace.environment(?!s)|environments.environment)(?!.*default)/i",
488479
"group": "20env@50"
489480
},
490-
{
491-
"command": "azure-dev.commands.cli.env-delete",
492-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.environments.environment(?!.*default)/i",
493-
"group": "10env@40"
494-
},
495481
{
496482
"command": "azure-dev.commands.cli.restore",
497483
"when": "viewItem =~ /ms-azuretools.azure-dev.views.workspace.(application|services|service(?!s))/i",
@@ -524,19 +510,9 @@
524510
},
525511
{
526512
"command": "azure-dev.commands.azureWorkspace.revealAzureResourceGroup",
527-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.workspace.application/i",
513+
"when": "viewItem =~ /ms-azuretools.azure-dev.views.(workspace.application|workspace.environment(?!s)|environments.environment)/i",
528514
"group": "50navigation@20"
529515
},
530-
{
531-
"command": "azure-dev.commands.azureWorkspace.revealAzureResourceGroup",
532-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.workspace.environment(?!s)/i",
533-
"group": "50navigation@20"
534-
},
535-
{
536-
"command": "azure-dev.commands.azureWorkspace.revealAzureResourceGroup",
537-
"when": "viewItem =~ /ms-azuretools.azure-dev.views.environments.environment/i",
538-
"group": "10env@50"
539-
},
540516
{
541517
"command": "azure-dev.views.environments.viewDotEnv",
542518
"when": "viewItem =~ /ms-azuretools.azure-dev.views.environments.environment/i",

‎ext/vscode/package.nls.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@
1919
"azure-dev.commands.cli.pipeline-config.title": "Configure DevOps Pipelines (pipeline config)",
2020
"azure-dev.commands.cli.install.title": "Install or Update Azure Developer CLI",
2121
"azure-dev.commands.cli.login.title": "Sign in with Azure Developer CLI",
22+
"azure-dev.commands.cli.extension-install.title": "Install Extension",
23+
"azure-dev.commands.cli.extension-uninstall.title": "Uninstall Extension",
24+
"azure-dev.commands.cli.extension-upgrade.title": "Upgrade Extension",
25+
"azure-dev.commands.addService.title": "Add Service...",
26+
"azure-dev.views.environments.toggleEnvVarVisibility.title": "Toggle Environment Variable Visibility",
27+
"azure-dev.views.workspace.toggleEnvVarVisibility.title": "Toggle Environment Variable Visibility",
28+
"azure-dev.views.environments.viewDotEnv.title": "View .env file",
29+
"azure-dev.views.templateTools.openReadme.title": "View README",
30+
"azure-dev.views.templateTools.openGitHub.title": "View on GitHub",
31+
"azure-dev.views.templateTools.initFromTemplateInline.title": "Initialize from Template",
32+
"azure-dev.views.templateTools.refresh.title": "Refresh",
2233
"azure-dev.commands.azureWorkspace.revealAzureResource.title": "Show Azure Resource",
2334
"azure-dev.commands.azureWorkspace.revealAzureResourceGroup.title": "Show Azure Resource Group",
2435
"azure-dev.commands.azureWorkspace.showInAzurePortal.title": "Show in Azure Portal",

‎ext/vscode/src/commands/azureWorkspace/wizard/RevealStep.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ export class RevealStep extends AzureWizardExecuteStep<RevealResourceWizardConte
5353
if (extension && !extension.isActive) {
5454
await extension.activate();
5555
ext.outputChannel.appendLog(vscode.l10n.t('Extension activated'));
56-
// Give it time to register its tree data provider
56+
// Delay to allow the extension to register its tree data provider.
57+
// The Azure Resources API doesn't provide an event for when provider registration completes.
5758
await new Promise(resolve => setTimeout(resolve, 1000));
5859
}
5960
}
@@ -67,6 +68,9 @@ export class RevealStep extends AzureWizardExecuteStep<RevealResourceWizardConte
6768
try {
6869
await vscode.commands.executeCommand('azureResourceGroups.refresh');
6970
ext.outputChannel.appendLog(vscode.l10n.t('Refresh command executed'));
71+
// Delay to allow the tree to fully populate after refresh.
72+
// The Azure Resources API doesn't expose an event for tree load completion,
73+
// so we use a delay as a pragmatic workaround.
7074
await new Promise(resolve => setTimeout(resolve, 1500));
7175
} catch (refreshError) {
7276
ext.outputChannel.appendLog(vscode.l10n.t('Refresh command not available or failed: {0}', refreshError instanceof Error ? refreshError.message : String(refreshError)));
@@ -84,6 +88,8 @@ export class RevealStep extends AzureWizardExecuteStep<RevealResourceWizardConte
8488
ext.outputChannel.appendLog(vscode.l10n.t('Revealing resource group first: {0}', rgResourceId));
8589
try {
8690
await api.resources.revealAzureResource(rgResourceId, { select: false, focus: false, expand: true });
91+
// Delay to allow the tree node to expand before revealing the child resource.
92+
// The revealAzureResource API returns before the tree UI fully updates.
8793
await new Promise(resolve => setTimeout(resolve, 1000));
8894
} catch (rgError) {
8995
ext.outputChannel.appendLog(vscode.l10n.t('Resource group reveal failed: {0}', rgError instanceof Error ? rgError.message : String(rgError)));
@@ -101,6 +107,8 @@ export class RevealStep extends AzureWizardExecuteStep<RevealResourceWizardConte
101107
// Try a second time if needed
102108
if (result === undefined) {
103109
ext.outputChannel.appendLog(vscode.l10n.t('First reveal returned undefined, trying again after delay...'));
110+
// Retry delay: the first reveal may fail if the tree hasn't finished loading.
111+
// A brief delay before retry often succeeds where the first attempt failed.
104112
await new Promise(resolve => setTimeout(resolve, 1000));
105113
const secondResult = await api.resources.revealAzureResource(azureResourceId, { select: true, focus: true, expand: true });
106114
ext.outputChannel.appendLog(vscode.l10n.t('Second attempt returned: {0}', String(secondResult)));

‎ext/vscode/src/commands/env.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,8 +353,6 @@ export async function refreshEnvironment(context: IActionContext, selectedItem?:
353353
}
354354
// Refresh standalone environments view
355355
void vscode.commands.executeCommand('azure-dev.views.environments.refresh');
356-
// Refresh workspace resource view
357-
void vscode.commands.executeCommand('azureWorkspace.refresh');
358356
});
359357
}
360358

‎ext/vscode/src/commands/registerCommands.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,15 @@ export function registerCommands(): void {
7070
// registerActivityCommand wraps a command callback with activity recording.
7171
// The command ID is automatically used as the telemetry event name by registerCommandAzUI.
7272
// For CLI task executions, telemetry is separately tracked via executeAsTask() with TelemetryId enum values.
73-
function registerActivityCommand(commandId: string, callback: CommandCallback, debounce?: number): void {
73+
function registerActivityCommand(commandId: string, callback: CommandCallback, debounce?: number, telemetryId?: string): void {
7474
registerCommandAzUI(
7575
commandId,
7676
// eslint-disable-next-line @typescript-eslint/no-explicit-any
7777
async(context: IActionContext, ...args: any[]) => {
7878
void ext.activitySvc.recordActivity();
7979
return callback(context, ...args);
8080
},
81-
debounce
81+
debounce,
82+
telemetryId
8283
);
8384
}

‎ext/vscode/src/telemetry/telemetryId.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ export enum TelemetryId {
8686

8787
WorkspaceViewApplicationResolve = 'azure-dev.views.workspace.application.resolve',
8888
WorkspaceViewEnvironmentResolve = 'azure-dev.views.workspace.environment.resolve',
89-
WorkspaceViewExtensionResolve = 'azure-dev.views.workspace.extension.resolve',
89+
ExtensionsViewResolve = 'azure-dev.views.extensions.resolve',
9090

9191
// Reported when diagnostics are provided on an azure.yaml document
9292
AzureYamlProvideDiagnostics = 'azure-dev.azureYaml.provideDiagnostics',

‎ext/vscode/src/test/suite/unit/azureDevTemplateProvider.test.ts‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -101,23 +101,6 @@ suite('AzureDevTemplateProvider', () => {
101101
expect(count, 'Count should be non-negative').to.be.at.least(0);
102102
});
103103

104-
test('caching works - second call is faster', async () => {
105-
// First call - fetches from network
106-
const start1 = Date.now();
107-
await provider.getTemplates();
108-
const duration1 = Date.now() - start1;
109-
110-
// Second call - uses cache
111-
const start2 = Date.now();
112-
await provider.getTemplates();
113-
const duration2 = Date.now() - start2;
114-
115-
// Cache should be significantly faster (at least 10x)
116-
// Note: This is a heuristic and may not always pass due to network conditions
117-
expect(duration2 < duration1 / 5 || duration2 < 10,
118-
`Second call should be faster (${duration2}ms) than first (${duration1}ms)`).to.be.true;
119-
});
120-
121104
test('forceRefresh parameter refreshes cache', async () => {
122105
// Load into cache
123106
const templates1 = await provider.getTemplates();

‎ext/vscode/src/test/suite/unit/templateToolsTreeDataProvider.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,6 @@ suite('TemplateToolsTreeDataProvider', () => {
175175
expect(
176176
templateItem.contextValue,
177177
'Should have template context value for inline menu actions'
178-
).to.equal('template');
178+
).to.equal('ms-azuretools.azure-dev.views.templateTools.template');
179179
});
180180
});

‎ext/vscode/src/views/environments/EnvironmentsTreeDataProvider.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
14
import * as vscode from 'vscode';
25
import { callWithTelemetryAndErrorHandling, IActionContext } from '@microsoft/vscode-azext-utils';
36
import { TelemetryId } from '../../telemetry/telemetryId';
@@ -199,8 +202,8 @@ export class EnvironmentsTreeDataProvider implements vscode.TreeDataProvider<Env
199202
item.tooltip = isVisible ? `${key}=${value}` : 'Click to view value';
200203
item.iconPath = new vscode.ThemeIcon('key');
201204
item.command = {
202-
command: 'azure-dev.views.environments.toggleVisibility',
203-
title: 'Toggle Visibility',
205+
command: 'azure-dev.views.environments.toggleEnvVarVisibility',
206+
title: 'Toggle Environment Variable Visibility',
204207
arguments: [item]
205208
};
206209

‎ext/vscode/src/views/extensions/ExtensionsTreeDataProvider.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export class ExtensionsTreeDataProvider implements vscode.TreeDataProvider<Exten
3636
return [];
3737
}
3838

39-
return await callWithTelemetryAndErrorHandling(TelemetryId.WorkspaceViewExtensionResolve, async (context) => {
39+
return await callWithTelemetryAndErrorHandling(TelemetryId.ExtensionsViewResolve, async (context) => {
4040
const extensions = await this.extensionProvider.getExtensionListResults(context);
4141
return extensions.map(ext => new ExtensionTreeItem(ext));
4242
}) ?? [];

0 commit comments

Comments
 (0)