-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add processFilter for remote attach process selection #14684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /* -------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All Rights Reserved. | ||
| * See 'LICENSE' in the project root for license information. | ||
| * ------------------------------------------------------------------------------------------ */ | ||
|
|
||
| export interface ProcessFilterItem { | ||
| label?: string; | ||
| description?: string; | ||
| detail?: string; | ||
| } | ||
|
|
||
| export function filterProcessItems<T extends ProcessFilterItem>(items: T[], processFilter?: unknown): T[] | undefined { | ||
| // The value comes from launch.json, so it is not guaranteed to be a string. | ||
| const trimmedFilter: string | undefined = typeof processFilter === 'string' ? processFilter.trim() : undefined; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨Copilot (agent135): [Moderate] Please preserve the configured regex here. Leading and trailing whitespace is valid regex syntax: for example, |
||
| if (!trimmedFilter) { | ||
| return undefined; | ||
| } | ||
|
|
||
| let processRegex: RegExp; | ||
| try { | ||
| processRegex = new RegExp(trimmedFilter); | ||
| } catch { | ||
| throw new Error(`Invalid processFilter regular expression: ${trimmedFilter}`); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨Copilot (agent135): [Minor] This newly introduced exception is user-facing through both remote-picker invocation paths, but its text is hard-coded English. Please route it through the existing |
||
| } | ||
|
|
||
| return items.filter((item: T) => { | ||
| const label: string = item.label ?? ""; | ||
| const description: string = item.description ?? ""; | ||
| const detail: string = item.detail ?? ""; | ||
| return processRegex.test(label) || processRegex.test(description) || processRegex.test(detail); | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| /* -------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All Rights Reserved. | ||
| * See 'LICENSE' in the project root for license information. | ||
| * ------------------------------------------------------------------------------------------ */ | ||
|
|
||
| import { deepStrictEqual, strictEqual, throws } from 'assert'; | ||
| import { describe, it } from 'mocha'; | ||
| import { filterProcessItems } from '../../src/Debugger/processFilter'; | ||
|
|
||
| interface TestProcessItem { | ||
| label?: string; | ||
| description?: string; | ||
| detail?: string; | ||
| id: string; | ||
| } | ||
|
|
||
| describe('Remote attach process filter', () => { | ||
| const processes: TestProcessItem[] = [ | ||
| { id: '101', label: 'root /usr/bin/my-daemon --serve', description: '101' }, | ||
| { id: '102', label: 'root /usr/bin/other-service', description: '102', detail: 'worker' }, | ||
| { id: '103', label: 'app /usr/bin/my-daemon --once', description: '103' } | ||
| ]; | ||
|
|
||
| it('returns undefined when filter is empty', () => { | ||
| strictEqual(filterProcessItems(processes, ''), undefined); | ||
| strictEqual(filterProcessItems(processes, ' '), undefined); | ||
| strictEqual(filterProcessItems(processes, undefined), undefined); | ||
| }); | ||
|
|
||
| it('returns undefined when filter is not a string', () => { | ||
| strictEqual(filterProcessItems(processes, 1234), undefined); | ||
| strictEqual(filterProcessItems(processes, true), undefined); | ||
| strictEqual(filterProcessItems(processes, {}), undefined); | ||
| }); | ||
|
|
||
| it('matches by label and description and detail', () => { | ||
| deepStrictEqual(filterProcessItems(processes, 'other-service')?.map(p => p.id), ['102']); | ||
| deepStrictEqual(filterProcessItems(processes, '^101$')?.map(p => p.id), ['101']); | ||
| deepStrictEqual(filterProcessItems(processes, 'worker')?.map(p => p.id), ['102']); | ||
| }); | ||
|
|
||
| it('returns multiple matches when regex matches more than one process', () => { | ||
| deepStrictEqual(filterProcessItems(processes, 'my-daemon')?.map(p => p.id), ['101', '103']); | ||
| }); | ||
|
|
||
| it('throws for invalid regular expression', () => { | ||
| throws(() => filterProcessItems(processes, '['), /Invalid processFilter regular expression/); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨Copilot (agent135): [Moderate] Please add this property to
Extension/tools/OptionsSchema.jsonunderCppdbgAttachOptionsand regenerate this file.OptionsSchema.jsonis the documented schema source, andgenerateOptionsSchema.tsreplaces the completecppdbgattach schema inpackage.jsonfrom it. As written, the nextyarn generate-options-schemaremovesprocessFilter, including its completion, validation, and documentation. This same source/generated drift previously required #14523 to resynchronize.