Skip to content

Commit 41184c2

Browse files
EhabYEhabY
authored
Fix command palette visibility with proper "when" clause placement (#692)
Move "when" clauses from command definitions to menus.commandPalette section. The fixes the visibility of commands when logged in, logged out, or connected to a remote deployment.
1 parent 99d1fab commit 41184c2

File tree

7 files changed

+113
-27
lines changed

7 files changed

+113
-27
lines changed

‎package.json‎

Lines changed: 60 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -192,67 +192,67 @@
192192
"commands": [
193193
{
194194
"command": "coder.login",
195-
"title": "Coder: Login"
195+
"title": "Login",
196+
"category": "Coder",
197+
"icon": "$(sign-in)"
196198
},
197199
{
198200
"command": "coder.logout",
199-
"title": "Coder: Logout",
200-
"when": "coder.authenticated",
201+
"title": "Logout",
202+
"category": "Coder",
201203
"icon": "$(sign-out)"
202204
},
203205
{
204206
"command": "coder.open",
205207
"title": "Open Workspace",
206-
"icon": "$(play)",
207-
"category": "Coder"
208+
"category": "Coder",
209+
"icon": "$(play)"
208210
},
209211
{
210212
"command": "coder.openFromSidebar",
211-
"title": "Coder: Open Workspace",
213+
"title": "Open Workspace",
214+
"category": "Coder",
212215
"icon": "$(play)"
213216
},
214217
{
215218
"command": "coder.createWorkspace",
216219
"title": "Create Workspace",
217220
"category": "Coder",
218-
"when": "coder.authenticated",
219221
"icon": "$(add)"
220222
},
221223
{
222224
"command": "coder.navigateToWorkspace",
223225
"title": "Navigate to Workspace Page",
224-
"when": "coder.authenticated",
226+
"category": "Coder",
225227
"icon": "$(link-external)"
226228
},
227229
{
228230
"command": "coder.navigateToWorkspaceSettings",
229231
"title": "Edit Workspace Settings",
230-
"when": "coder.authenticated",
232+
"category": "Coder",
231233
"icon": "$(settings-gear)"
232234
},
233235
{
234236
"command": "coder.workspace.update",
235-
"title": "Coder: Update Workspace",
236-
"when": "coder.workspace.updatable"
237+
"title": "Update Workspace",
238+
"category": "Coder"
237239
},
238240
{
239241
"command": "coder.refreshWorkspaces",
240242
"title": "Refresh Workspace",
241243
"category": "Coder",
242-
"icon": "$(refresh)",
243-
"when": "coder.authenticated"
244+
"icon": "$(refresh)"
244245
},
245246
{
246247
"command": "coder.viewLogs",
247248
"title": "Coder: View Logs",
248-
"icon": "$(list-unordered)",
249-
"when": "coder.authenticated"
249+
"icon": "$(list-unordered)"
250250
},
251251
{
252252
"command": "coder.openAppStatus",
253-
"title": "Coder: Open App Status",
254-
"icon": "$(robot)",
255-
"when": "coder.authenticated"
253+
"title": "Open App Status",
254+
"category": "Coder",
255+
"icon": "$(robot)"
256256
},
257257
{
258258
"command": "coder.searchMyWorkspaces",
@@ -275,8 +275,44 @@
275275
"menus": {
276276
"commandPalette": [
277277
{
278-
"command": "coder.debug.listDeployments",
279-
"when": "coder.devMode"
278+
"command": "coder.login",
279+
"when": "!coder.authenticated"
280+
},
281+
{
282+
"command": "coder.logout",
283+
"when": "coder.authenticated"
284+
},
285+
{
286+
"command": "coder.createWorkspace",
287+
"when": "coder.authenticated"
288+
},
289+
{
290+
"command": "coder.navigateToWorkspace",
291+
"when": "coder.workspace.connected"
292+
},
293+
{
294+
"command": "coder.navigateToWorkspaceSettings",
295+
"when": "coder.workspace.connected"
296+
},
297+
{
298+
"command": "coder.workspace.update",
299+
"when": "coder.workspace.updatable"
300+
},
301+
{
302+
"command": "coder.refreshWorkspaces",
303+
"when": "coder.authenticated"
304+
},
305+
{
306+
"command": "coder.viewLogs",
307+
"when": "true"
308+
},
309+
{
310+
"command": "coder.openAppStatus",
311+
"when": "false"
312+
},
313+
{
314+
"command": "coder.open",
315+
"when": "coder.authenticated"
280316
},
281317
{
282318
"command": "coder.openFromSidebar",
@@ -289,6 +325,10 @@
289325
{
290326
"command": "coder.searchAllWorkspaces",
291327
"when": "false"
328+
},
329+
{
330+
"command": "coder.debug.listDeployments",
331+
"when": "coder.devMode"
292332
}
293333
],
294334
"view/title": [

‎src/api/utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import fs from "fs/promises";
1+
import fs from "node:fs/promises";
22
import { ProxyAgent } from "proxy-agent";
33
import { type WorkspaceConfiguration } from "vscode";
44

‎src/commands.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ export class Commands {
242242
*
243243
* Otherwise, the currently connected workspace is used (if any).
244244
*/
245-
public async navigateToWorkspace(item: OpenableTreeItem) {
245+
public async navigateToWorkspace(item?: OpenableTreeItem) {
246246
if (item) {
247247
const baseUrl = this.requireExtensionBaseUrl();
248248
const workspaceId = createWorkspaceIdentifier(item.workspace);
@@ -266,7 +266,7 @@ export class Commands {
266266
*
267267
* Otherwise, the currently connected workspace is used (if any).
268268
*/
269-
public async navigateToWorkspaceSettings(item: OpenableTreeItem) {
269+
public async navigateToWorkspaceSettings(item?: OpenableTreeItem) {
270270
if (item) {
271271
const baseUrl = this.requireExtensionBaseUrl();
272272
const workspaceId = createWorkspaceIdentifier(item.workspace);

‎src/core/contextManager.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const CONTEXT_DEFAULTS = {
44
"coder.authenticated": false,
55
"coder.isOwner": false,
66
"coder.loaded": false,
7+
"coder.workspace.connected": false,
78
"coder.workspace.updatable": false,
89
} as const;
910

‎src/remote/remote.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,7 @@ export class Remote {
556556
throw ex;
557557
}
558558

559+
this.contextManager.set("coder.workspace.connected", true);
559560
this.logger.info("Remote setup complete");
560561

561562
// Returning the URL and token allows the plugin to authenticate its own

‎src/remote/sshProcess.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,9 @@ export class SshProcessMonitor implements vscode.Disposable {
256256
const targetPid = this.currentPid;
257257
while (!this.disposed && this.currentPid === targetPid) {
258258
try {
259-
const logFiles = await fs.readdir(logDir);
260-
logFiles.sort().reverse();
259+
const logFiles = (await fs.readdir(logDir))
260+
.sort((a, b) => a.localeCompare(b))
261+
.reverse();
261262
const logFileName = logFiles.find(
262263
(file) =>
263264
file === `${targetPid}.log` || file.endsWith(`-${targetPid}.log`),
@@ -420,7 +421,7 @@ async function findRemoteSshLogPath(
420421
const dirs = await fs.readdir(logsParentDir);
421422
const outputDirs = dirs
422423
.filter((d) => d.startsWith("output_logging_"))
423-
.sort()
424+
.sort((a, b) => a.localeCompare(b))
424425
.reverse();
425426

426427
if (outputDirs.length > 0) {

‎test/unit/remote/sshProcess.test.ts‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,27 @@ describe("SshProcessMonitor", () => {
127127
expect(find).toHaveBeenCalledWith("port", 33333);
128128
});
129129

130+
it("sorts output_logging_ directories using localeCompare for consistent ordering", async () => {
131+
// localeCompare differs from default sort() for mixed case
132+
vol.fromJSON({
133+
"/logs/output_logging_a/1-Remote - SSH.log": "-> socksPort 11111 ->",
134+
"/logs/output_logging_Z/1-Remote - SSH.log": "-> socksPort 22222 ->",
135+
});
136+
137+
mockReaddirOrder("/logs", [
138+
"output_logging_a",
139+
"output_logging_Z",
140+
"window1",
141+
]);
142+
143+
const monitor = createMonitor({ codeLogDir: "/logs/window1" });
144+
await waitForEvent(monitor.onPidChange);
145+
146+
// With localeCompare: ["a", "Z"] -> reversed -> "Z" first (port 22222)
147+
// With plain sort(): ["Z", "a"] -> reversed -> "a" first (port 11111)
148+
expect(find).toHaveBeenCalledWith("port", 22222);
149+
});
150+
130151
it("falls back to output_logging_ when extension folder has no SSH log", async () => {
131152
// Extension folder exists but doesn't have Remote SSH log
132153
vol.fromJSON({
@@ -301,6 +322,28 @@ describe("SshProcessMonitor", () => {
301322

302323
expect(logPath).toBe("/proxy-logs/2024-01-03-999.log");
303324
});
325+
326+
it("sorts log files using localeCompare for consistent cross-platform ordering", async () => {
327+
// localeCompare differs from default sort() for mixed case
328+
vol.fromJSON({
329+
"/logs/ms-vscode-remote.remote-ssh/1-Remote - SSH.log":
330+
"-> socksPort 12345 ->",
331+
"/proxy-logs/a-999.log": "",
332+
"/proxy-logs/Z-999.log": "",
333+
});
334+
335+
mockReaddirOrder("/proxy-logs", ["a-999.log", "Z-999.log"]);
336+
337+
const monitor = createMonitor({
338+
codeLogDir: "/logs/window1",
339+
proxyLogDir: "/proxy-logs",
340+
});
341+
const logPath = await waitForEvent(monitor.onLogFilePathChange);
342+
343+
// With localeCompare: ["a", "Z"] -> reversed -> "Z" first
344+
// With plain sort(): ["Z", "a"] -> reversed -> "a" first (WRONG)
345+
expect(logPath).toBe("/proxy-logs/Z-999.log");
346+
});
304347
});
305348

306349
describe("network status", () => {
@@ -483,7 +526,7 @@ function mockReaddirOrder(dirPath: string, files: string[]): void {
483526
if (path === dirPath) {
484527
return Promise.resolve(files);
485528
}
486-
return originalReaddir(path) as Promise<string[]>;
529+
return originalReaddir(path);
487530
};
488531
vi.spyOn(fsPromises, "readdir").mockImplementation(
489532
mockImpl as typeof fsPromises.readdir,

0 commit comments

Comments
 (0)