Repository navigation
fix: skipFiles ignoring negations of absolute paths and node internals - #2439
Open
Feldt (Feldt) wants to merge 1 commit into
Open
Feldt (Feldt) wants to merge 1 commit into
Feldt (Feldt) wants to merge 1 commit into
Conversation
preprocessAuthoredGlobs turns absolute skipFiles entries into file URLs,
but it called isAbsolute() on the raw pattern. The leading "!" makes a
negated absolute path look relative, so "!${workspaceFolder}/foo/**"
stayed a bare path while the positive globs became file:/// URLs, and
the negation never matched a script. Convert the glob without its "!"
and put the "!" back afterwards.
Negated node internals ("!<node_internals>/events") were dropped for
the same reason: preprocessNodeInternals only matched entries that
start with "<node_internals>/", and preprocessAuthoredGlobs skips any
entry with the token. Keep the "!" on the node internals glob, which
micromatch applies as a negation, and only use the list when at least
one entry has no leading "!", so a lone negation still skips nothing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Feldt (@Feldt) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Negated absolute paths in
skipFileshave no effect. Withscripts in
node_modules/my-libare still skipped. The same happens with a relative positive entry (**/node_modules/**) and an absolute negation. Relative negations such as!**/node_modules/my-lib/**work. Negated node internals are ignored too:["<node_internals>/**", "!<node_internals>/events"]still skipsnode:events.Absolute paths.
preprocessAuthoredGlobsturns absolute entries intofile:///URLs, but it callsurlUtils.isAbsolute()on the raw pattern. The leading!makes a negated entry look relative, so it stays a bare path (!/proj/node_modules/my-lib/**) while the positive entry becomesfile:///proj/node_modules/**. The lookahead built from it,^(?!\/proj\/node_modules\/my-lib\/)file:\/\/\/proj\/node_modules\/, can never match afile:///URL, so the exception is dropped, both inisScriptSkippedand in the patterns sent toDebugger.setBlackboxPatterns. This came in with #1422, which added the file URL conversion.This change converts the glob without its
!(throughrebaseLocalToRemoteas well, so negations get the same remote mapping as positive entries) and then puts the!back. The result has the same!file:///...form thatsetIsUrlBlackboxSkippedalready writes when a file is toggled.Node internals.
preprocessNodeInternalsonly takes entries that start with<node_internals>/, andpreprocessAuthoredGlobsdrops every entry that contains the token, so!<node_internals>/eventswas dropped by both. This part is older than #1422. The regex now also accepts a leading!and keeps it on the glob, which micromatch applies as a negation. The list is only used when at least one<node_internals>entry has no leading!. On its own, micromatch would read a lone negation as "every other module", while today it skips nothing, so that case stays as it is. A negation matches the same names as the positive entry would:<node_internals>/eventsfornode:events, and<node_internals>/events.json Node 14 and older.src/adapter/scriptSkipper/scriptSkipper.test.ts. It builds aScriptSkipperwith a fake CDP and checksisScriptSkippedand the regexes sent tosetBlackboxPatterns.expected true to be false) and pass with this change: absolute entry with absolute negation, relative entry with absolute negation,!<node_internals>/events(node:names), and!<node_internals>/events.js(Node 14 names).!<node_internals>/...entry (still skips nothing), and<node_internals>/!(events), whose glob starts with!itself.npm run test:unit: 312 passing. With onlyimplementation.tsreverted to main, the four cases above are the only failures.npm run test:types, plusdprint checkandeslint --max-warnings=0on the touched files: all clean.I prepared this change with an AI coding agent (Claude Code). I am responsible for it and will answer review comments.
🤖 Generated with Claude Code