Skip to content

Accept relative patterns in NotebookDocumentFilter.is - #1931

Merged
Dirk Bäumer (dbaeumer) merged 1 commit into
microsoft:mainfrom
NgoQuocViet2001:fix-notebook-filter-relative-pattern
Oct 30, 2026
Merged

Dirk Bäumer (dbaeumer) merged 1 commit into
microsoft:mainfrom
NgoQuocViet2001:fix-notebook-filter-relative-pattern

Conversation

@NgoQuocViet2001

Copy link
Copy Markdown
Contributor

NotebookDocumentFilter.is still only accepts a string pattern, although #1519 changed the pattern of all notebook filters to GlobPattern. The client converter already uses asGlobPattern for it, and TextDocumentFilter.is moved to GlobPattern.is in #1516. A notebook filter identified only by a relative pattern is rejected:

const rel = { baseUri: 'file:///ws', pattern: '**/*.ipynb' };
NotebookDocumentFilter.is({ pattern: '**/*.ipynb' }); // true
NotebookDocumentFilter.is({ pattern: rel });          // false, expected true

Because of this, NotebookCellTextDocumentFilter.is and DocumentSelector.is also return false. The client's asDocumentSelector then turns { notebook: { pattern: rel }, language: 'python' } into the plain text filter { language: 'python' }, or drops it when there is no language. This change uses GlobPattern.is for the notebook filter pattern, the same as TextDocumentFilter.is.

  • Ran: npm run compile:protocol and the new protocol/src/node/test/documentFilter.test.ts. Before the change 4 of its 7 tests fail; after it all pass.
  • Ran: npm --prefix protocol run test:node (12 passing) and npm --prefix protocol run lint (clean).
  • Checked: string patterns still pass. {}, { pattern: 1 }, { pattern: {} } and an invalid baseUri are still rejected. A plain text filter with a relative pattern is still not treated as a notebook cell filter.

Copilot AI balanced review requested due to automatic review settings October 29, 2026 18:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain for the change enabling relative glob patterns in notebook filter validation.

No open issues 🎉

What changed in this PR

Aligns notebook filter validation with the existing GlobPattern type support from #1519, allowing relative patterns to pass through notebook cell and document selector guards.

Changes:

  • Uses GlobPattern.is instead of a string-only check in NotebookDocumentFilter.is.
  • Adds regression tests for relative patterns, invalid inputs, string compatibility, and dependent selector guards.
File Description
protocol/​src/​node/​test/​documentFilter.test.ts Adds coverage for notebook filters and dependent type guards.
protocol/​src/​common/​protocol.ts Accepts relative glob patterns in notebook filter validation.

Review effort: Balanced

@dbaeumer
Dirk Bäumer (dbaeumer) merged commit c331cf0 into microsoft:main Oct 30, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants