Skip to content

Commit

Permalink
Merge pull request #12881 from OfficeDev/huajie/pick-debug-port
Browse files Browse the repository at this point in the history
fix: pick debug port improvement
  • Loading branch information
MSFT-yiz authored Dec 10, 2024
2 parents 878c546 + ed6062c commit 517e4ab
Show file tree
Hide file tree
Showing 11 changed files with 147 additions and 57 deletions.
2 changes: 1 addition & 1 deletion packages/api/src/qm/question.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ export interface MultiSelectQuestion extends UserInputQuestion {
/**
* The default selected `id` array of the option item
*/
default?: string[] | LocalFunc<string[] | undefined>;
default?: string[] | LocalFunc<string[] | undefined> | "none" | "all";

/**
* This config only works for option items with `OptionItem[]` type. If `returnObject` is true, the answer value is an array of `OptionItem` objects; otherwise, the answer value is an array of `id` strings.
Expand Down
4 changes: 2 additions & 2 deletions packages/api/src/qm/ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface UIConfig<T> {
/**
* default input value
*/
default?: T | (() => Promise<T>);
default?: T | (() => Promise<T>) | string;

/**
* A function that will be called to validate input and to give a hint to the user.
Expand Down Expand Up @@ -110,7 +110,7 @@ export interface MultiSelectConfig extends UIConfig<string[]> {
*/
options: StaticOptions | (() => Promise<StaticOptions>);

default?: string[] | (() => Promise<string[]>);
default?: string[] | (() => Promise<string[]>) | "none" | "all";

/**
* This config only works for option items with `OptionItem[]` type. If `returnObject` is true, the answer value is an array of `OptionItem` objects; otherwise, the answer value is an array of `id` strings.
Expand Down
7 changes: 7 additions & 0 deletions packages/cli/src/userInteraction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,13 @@ class CLIUserInteraction implements UserInteraction {
}
}
}
if (config.default === "all") {
config.default = (config.options as StaticOptions).map((o) =>
typeof o === "string" ? o : o.id
);
} else if (config.default === "none") {
config.default = [];
}
const [choices, defaultValue] = this.toChoices(
config.options as StaticOptions,
config.default as string[]
Expand Down
1 change: 1 addition & 0 deletions packages/cli/tests/unit/engine.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ describe("CLI Engine", () => {

beforeEach(() => {
sandbox.stub(process, "exit");
sandbox.stub(CliTelemetry, "flush").resolves();
});

afterEach(() => {
Expand Down
39 changes: 39 additions & 0 deletions packages/cli/tests/unit/ui2.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,45 @@ describe("UserInteraction(CLI) 2", () => {
const result = await UI.selectOptions(config);
assert.isTrue(result.isErr());
});
it("success with default=all", async () => {
const config: MultiSelectConfig = {
name: "test",
title: "test",
options: async () => {
return ["a", "b", "c"];
},
default: "all",
};
const result = await UI.selectOptions(config);
assert.isTrue(result.isOk());
});
it("success with default=all", async () => {
const config: MultiSelectConfig = {
name: "test",
title: "test",
options: async () => {
return [
{ id: "a", label: "a" },
{ id: "b", label: "b" },
];
},
default: "all",
};
const result = await UI.selectOptions(config);
assert.isTrue(result.isOk());
});
it("success with default=none", async () => {
const config: MultiSelectConfig = {
name: "test",
title: "test",
options: async () => {
return ["a", "b", "c"];
},
default: "none",
};
const result = await UI.selectOptions(config);
assert.isTrue(result.isOk());
});
});

describe("selectOption", () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/fx-core/resource/package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -930,5 +930,6 @@
"error.dep.NodejsNotLtsError": "Node.js (%s) is not a LTS version (%s). Go to https://nodejs.org to install LTS Node.js.",
"error.dep.NodejsNotRecommendedError": "Node.js (%s) is not the officially supported version (%s). Your project may continue to work but we recommend to install the supported version. The supported node versions are specified in the package.json. Go to https://nodejs.org to install a supported Node.js.",
"error.dep.VxTestAppInvalidInstallOptionsError": "Invalid argument for video extensibility test app prerequisites checker. Please review tasks.json file to ensure all arguments are correctly formatted and valid.",
"error.dep.VxTestAppValidationError": "Unable to validate video extensibility test app after installation."
"error.dep.VxTestAppValidationError": "Unable to validate video extensibility test app after installation.",
"error.dep.FindProcessError": "Unable to find process(es) by pid or port. %s"
}
17 changes: 16 additions & 1 deletion packages/fx-core/src/error/depCheck.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { UserError, UserErrorOptions } from "@microsoft/teamsfx-api";
import { SystemError, UserError, UserErrorOptions } from "@microsoft/teamsfx-api";
import { getDefaultString, getLocalizedString } from "../common/localizeUtils";
import { ErrorCategory } from "./types";

Expand Down Expand Up @@ -148,3 +148,18 @@ export class DepsCheckerError extends UserError {
super(errorOptions);
}
}

export class FindProcessError extends SystemError {
constructor(error: Error, source?: string) {
const key = "error.dep.FindProcessError";
const errorOptions: UserErrorOptions = {
source: source || "core",
name: "FindProcessError",
error: error,
message: getDefaultString(key, error.message),
displayMessage: getLocalizedString(key, error.message),
categories: [ErrorCategory.Internal],
};
super(errorOptions);
}
}
12 changes: 12 additions & 0 deletions packages/fx-core/tests/error/error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { InvalidYamlSchemaError } from "../../src/error/yml";
import { getLocalizedString } from "../../src/common/localizeUtils";
import {
CopilotDisabledError,
FindProcessError,
NodejsNotLtsError,
PortsConflictError,
SideloadingDisabledError,
Expand Down Expand Up @@ -331,4 +332,15 @@ describe("DeveloperPortalAPIFailed error", function () {
assert.isTrue(!!error.displayMessage);
assert.isFalse(!!error.helpLink);
});

describe("FindProcessError", function () {
it("happy", () => {
const err = new FindProcessError(new Error(), "test");
assert.deepEqual(err.source, "test");
});
it("happy no source", () => {
const err = new FindProcessError(new Error());
assert.deepEqual(err.source, "core");
});
});
});
111 changes: 61 additions & 50 deletions packages/vscode-extension/src/debug/depsChecker/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
DepsManager,
DepsType,
ErrorCategory,
FindProcessError,
LocalEnvManager,
PackageService,
PortsConflictError,
Expand Down Expand Up @@ -178,60 +179,70 @@ async function selectPortsToKill(
LocalDebugPorts.terminateButton = selectButton!;

if (selectButton === "Terminate Process") {
const process2ports = new Map<number, number[]>();
for (const port of portsInUse) {
const processList = await find("port", port);
if (processList.length > 0) {
const process = processList[0];
const ports = process2ports.get(process.pid);
if (ports) {
ports.push(port);
} else {
process2ports.set(process.pid, [port]);
const loadOptions = async () => {
try {
const process2ports = new Map<number, number[]>();
for (const port of portsInUse) {
const processList = await find("port", port);
if (processList.length > 0) {
const process = processList[0];
const ports = process2ports.get(process.pid);
if (ports) {
ports.push(port);
} else {
process2ports.set(process.pid, [port]);
}
}
}
if (process2ports.size > 0) {
const options: OptionItem[] = [];
for (const processId of process2ports.keys()) {
const ports = process2ports.get(processId);
LocalDebugPorts.process2conflictPorts[processId] = ports!;
const findList = await find("pid", processId);
if (findList.length > 0) {
const processInfo = findList[0].cmd;
options.push({
id: `${processId}`,
label: `'${String(processInfo)}' (${processId}) occupies port(s): ${ports!.join(
","
)}`,
data: processInfo,
});
}
}
globalOptions = options;
return options;
}
return [];
} catch (e) {
throw new FindProcessError(e, ExtensionSource);
}
};

let globalOptions: OptionItem[] = [];
const res = await VS_CODE_UI.selectOptions({
title: "Select process(es) to terminate",
name: "select_processes",
options: loadOptions,
default: "all",
});
if (res.isErr()) {
return err(res.error);
}
if (process2ports.size > 0) {
const options: OptionItem[] = [];
for (const processId of process2ports.keys()) {
const ports = process2ports.get(processId);
LocalDebugPorts.process2conflictPorts[processId] = ports!;
const findList = await find("pid", processId);
if (findList.length > 0) {
const processInfo = findList[0].cmd;
options.push({
id: `${processId}`,
label: `'${String(processInfo)}' (${processId}) occupies port(s): ${ports!.join(",")}`,
data: processInfo,
});
}
if (res.isOk() && res.value.type === "success") {
const processIds = res.value.result as string[];
LocalDebugPorts.terminateProcesses = processIds;
for (const processId of processIds) {
await processUtil.killProcess(parseInt(processId));
}
const res = await VS_CODE_UI.selectOptions({
title: "Select process(es) to terminate",
name: "select_processes",
options,
default: options.map((o) => o.id),
});
if (res.isOk() && res.value.type === "success") {
const processIds = res.value.result as string[];
LocalDebugPorts.terminateProcesses = processIds;
for (const processId of processIds) {
await processUtil.killProcess(parseInt(processId));
}
if (processIds.length > 0) {
const processInfo = options
.filter((o) => processIds.includes(o.id))
.map((o) => `'${o.data as string}' (${o.id})`)
.join(", ");
void VS_CODE_UI.showMessage(
"info",
`Process(es) ${processInfo} have been killed.`,
false
);
return ok(undefined);
}
} else {
LocalDebugPorts.terminateProcesses = [];
if (processIds.length > 0) {
const processInfo = globalOptions
.filter((o) => processIds.includes(o.id))
.map((o) => `'${o.data as string}' (${o.id})`)
.join(", ");
void VS_CODE_UI.showMessage("info", `Process(es) ${processInfo} have been killed.`, false);
return ok(undefined);
}
}
} else if (selectButton === "Learn More") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export async function sendDebugAllEvent(
};

const closedPorts: number[] = [];
for (const port of globalVariables.LocalDebugPorts.checkPorts) {
for (const port of globalVariables.LocalDebugPorts.conflictPorts) {
const port2 = await detectPort(port);
if (port2 === port) {
closedPorts.push(port);
Expand Down
6 changes: 5 additions & 1 deletion packages/vscode-ui/src/ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,10 @@ export class VSCodeUI implements UserInteraction {
}
if (typeof config.default === "function") {
defaultValue = await config.default();
} else if (config.default === "all") {
defaultValue = options.map((o) => (typeof o === "string" ? o : o.id));
} else if (config.default === "none") {
defaultValue = [];
} else {
defaultValue = config.default || [];
}
Expand Down Expand Up @@ -779,7 +783,7 @@ export class VSCodeUI implements UserInteraction {
return this.selectFileInQuickPick(
config,
"files",
config.default ? config.default.join(";") : undefined
config.default && typeof config.default !== "string" ? config.default.join(";") : undefined
);
}

Expand Down

0 comments on commit 517e4ab

Please sign in to comment.