Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Apply glob negations if glob is navigated outside workspace folder #1479

Closed
connor4312 opened this issue Dec 14, 2022 · 2 comments · Fixed by #1486
Closed

Apply glob negations if glob is navigated outside workspace folder #1479

connor4312 opened this issue Dec 14, 2022 · 2 comments · Fixed by #1486
Assignees
@connor4312
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone
January 2023

Comments

@connor4312
Copy link
Member

The glob behavior is something we can improve, automatically applying it if we see a relative pattern navigates outside the workspace folder.

microsoft/vscode#169033
@connor4312 connor4312 added the bug Issue identified by VS Code Team member as probable bug label Dec 14, 2022
@connor4312 connor4312 self-assigned this Dec 14, 2022
connor4312 added a commit that referenced this issue Dec 15, 2022
@connor4312
fix: better handling of multiple/relative outFiles
f084419 
Fixes #1479
Relates to microsoft/vscode#168635

Previously, we naively joined all incoming globs together and ran a
single search for them. However, there are two problems:

- vscode's findTextInFiles doesn't really support multiple globs
- **-prefixed negations, like `!**/node_modules/**`, should apply
  everywhere, but globbing behavior is to only apply that to the base
  path, which led to confusion (#1479)

This PR redoes the logic. If there are N positive globs, we'll do N
searches in parallel, and apply relevant negations to each one.
@connor4312 connor4312 mentioned this issue Dec 15, 2022
Merged
@connor4312 connor4312 added this to the January 2023 milestone Dec 16, 2022
@andreamah andreamah added the verification-steps-needed Steps to verify are needed for verification label Jan 26, 2023
@andreamah
Copy link

are there some verif steps for this?

@connor4312
Copy link
Member Author

Use the steps in microsoft/vscode#169033, but before step 5, modify the launch.json by adding the following property to the Attach by Process ID configuration:

"outFiles": [
	"${workspaceFolder}/**/*.js",
	"${workspaceFolder:common-external}/**/*.js",
	"!**/node_modules/**"
],

Then, you should be able to have breakpoints bind in all the files.

@connor4312 connor4312 removed the verification-steps-needed Steps to verify are needed for verification label Jan 26, 2023
@roblourens roblourens added the verified Verification succeeded label Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@connor4312 connor4312

Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Milestone
January 2023
Development

Successfully merging a pull request may close this issue.

fix: better handling of multiple/relative outFiles microsoft/vscode-js-debug
3 participants
@roblourens @connor4312 @andreamah