-
Notifications
You must be signed in to change notification settings - Fork 428
Allow Exact Match For Filters #1649
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 |
|---|---|---|
|
|
@@ -490,25 +490,35 @@ export class JavaDebugConfigurationProvider implements vscode.DebugConfiguration | |
| const excludes: Map<string, boolean> = new Map<string, boolean>(); | ||
| for (const p of paths) { | ||
| if (p.startsWith("!")) { | ||
| let exclude = p.substr(1); | ||
| let exclude = p.slice(1); | ||
| let isDirect: boolean; | ||
|
|
||
| if (/[\\/]$/.test(exclude)) { | ||
|
Contributor
Author
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. before normalization, and using the exact code-block suggested, per feedback! |
||
| exclude = exclude.slice(0, -1); | ||
| isDirect = true; | ||
| } else { | ||
| isDirect = this.isFilePath(exclude); | ||
| } | ||
|
|
||
| if (!path.isAbsolute(exclude)) { | ||
| exclude = path.join(folder?.uri.fsPath || "", exclude); | ||
| } | ||
|
|
||
| // use Uri to normalize the fs path | ||
| excludes.set(vscode.Uri.file(exclude).fsPath, this.isFilePath(exclude)); | ||
| excludes.set(vscode.Uri.file(exclude).fsPath, isDirect); | ||
| continue; | ||
| } | ||
|
|
||
| result.push(vscode.Uri.file(p).fsPath); | ||
| } | ||
|
|
||
| return result.filter((r) => { | ||
| for (const [excludedPath, isFile] of excludes.entries()) { | ||
| if (isFile && r === excludedPath) { | ||
| for (const [excludedPath, isDirect] of excludes.entries()) { | ||
| if (isDirect && r === excludedPath) { | ||
|
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. Could you normalize trailing separators consistently on both sides before this exact comparison? Currently, |
||
| return false; | ||
| } | ||
|
|
||
| if (!isFile && r.startsWith(excludedPath)) { | ||
| if (!isDirect && r.startsWith(excludedPath)) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import * as assert from "assert"; | ||
| import * as fs from "fs"; | ||
| import * as os from "os"; | ||
| import * as path from "path"; | ||
| import * as vscode from "vscode"; | ||
|
|
||
| import { JavaDebugConfigurationProvider } from "../src/configurationProvider"; | ||
|
|
||
| interface TestWorkspace { | ||
| root: string; | ||
| folder: vscode.WorkspaceFolder; | ||
| libDir: string; | ||
| jarPath: string; | ||
| } | ||
|
|
||
| type FilterExcluded = ( | ||
| folder: vscode.WorkspaceFolder | undefined, | ||
| paths: string[], | ||
| ) => Promise<string[]>; | ||
|
|
||
| function createTestWorkspace(): TestWorkspace { | ||
| const root = fs.mkdtempSync(path.join(os.tmpdir(), "java-debug-cp-test-")); | ||
| const libDir = path.join(root, "lib"); | ||
| fs.mkdirSync(libDir); | ||
| const jarPath = path.join(libDir, "foo.jar"); | ||
| fs.writeFileSync(jarPath, ""); | ||
| return { | ||
| root, | ||
| folder: { | ||
| uri: vscode.Uri.file(root), | ||
| name: "test-workspace", | ||
| index: 0, | ||
| }, | ||
| libDir, | ||
| jarPath, | ||
| }; | ||
| } | ||
|
|
||
| function getFilterExcluded(provider: JavaDebugConfigurationProvider): FilterExcluded { | ||
| return (provider as unknown as { filterExcluded: FilterExcluded }).filterExcluded.bind(provider); | ||
| } | ||
|
|
||
| suite("JavaDebugConfigurationProvider", () => { | ||
| const workspaces: TestWorkspace[] = []; | ||
|
|
||
| suiteSetup(() => { | ||
| // configurationProvider requires ../package.json relative to out/src/ | ||
| const outPackageJson = path.join(__dirname, "../package.json"); | ||
| if (!fs.existsSync(outPackageJson)) { | ||
| fs.copyFileSync(path.join(__dirname, "../../package.json"), outPackageJson); | ||
| } | ||
| }); | ||
|
|
||
| teardown(() => { | ||
| while (workspaces.length > 0) { | ||
| const workspace = workspaces.pop()!; | ||
| fs.rmSync(workspace.root, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| suite("filterExcluded exact-match exclusions", () => { | ||
| async function assertExactDirectoryExclusion( | ||
| excludeSuffix: "\\" | "/", | ||
| label: string, | ||
| ): Promise<void> { | ||
| const workspace = createTestWorkspace(); | ||
| workspaces.push(workspace); | ||
|
|
||
| const libDirFs = vscode.Uri.file(workspace.libDir).fsPath; | ||
| const jarFs = vscode.Uri.file(workspace.jarPath).fsPath; | ||
| const filterExcluded = getFilterExcluded(new JavaDebugConfigurationProvider()); | ||
| const result = await filterExcluded(workspace.folder, [ | ||
| libDirFs, | ||
| jarFs, | ||
| `!${workspace.libDir}${excludeSuffix}`, | ||
| ]); | ||
|
|
||
| assert.deepStrictEqual( | ||
| result, | ||
| [jarFs], | ||
| `${label}: trailing slash should exact-exclude only the directory entry, not paths beneath it`, | ||
| ); | ||
| } | ||
|
|
||
| test("treats a trailing backslash as an exact match (Windows-style paths)", async () => { | ||
| await assertExactDirectoryExclusion("\\", "Windows-style"); | ||
| }); | ||
|
|
||
| test("treats a trailing forward slash as an exact match (Linux-style paths)", async () => { | ||
| await assertExactDirectoryExclusion("/", "Linux-style"); | ||
| }); | ||
| }); | ||
| }); |
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.
This should say “A trailing slash or backslash will treat the path as an exact match.” The current wording suggests that any slash triggers exact matching, while the implementation only checks the final character. Please make the same change to the
classPaths.excludedescription.