Fix placeholder filtering to respect exclude patterns (#1726)

Author: riccardoferretti

packages/foam-core/src/index.ts

@@ -22,7 +22,7 @@ export {
   AlwaysIncludeMatcher,
   SubstringExcludeMatcher,
 } from './services/datastore';
-export { GlobMatcher } from './services/glob-matcher';
+export { GlobMatcher, PlaceholderMatcher } from './services/glob-matcher';
 export type { GlobMatcherRoot } from './services/glob-matcher';
 export { createMarkdownParser, getLinkDefinitions, getBlockFor } from './services/markdown-parser';
 export type {

packages/foam-core/src/services/glob-matcher.test.ts

@@ -1,4 +1,4 @@
-import { GlobMatcher } from './glob-matcher';
+import { GlobMatcher, PlaceholderMatcher } from './glob-matcher';
 import { URI } from '../model/uri';
 
 const root = URI.file('/workspace');
@@ -142,3 +142,47 @@ describe('GlobMatcher', () => {
     expect(matcher.isMatch(root.joinPath('brand/new/note.md'))).toBeTruthy();
   });
 });
+
+describe('PlaceholderMatcher', () => {
+  it('keeps a wikilink placeholder, which lies under no root', () => {
+    const matcher = new PlaceholderMatcher([{ uri: root, exclude: [] }]);
+
+    expect(matcher.isMatch(URI.placeholder('missing-note'))).toBeTruthy();
+  });
+
+  it('hides a placeholder under an excluded folder, relative to its root', () => {
+    const matcher = new PlaceholderMatcher([
+      { uri: root, exclude: ['journal/**'] },
+    ]);
+
+    expect(
+      matcher.isMatch(URI.placeholder('/workspace/journal/2024-01-01.md'))
+    ).toBeFalsy();
+    expect(
+      matcher.isMatch(URI.placeholder('/workspace/Journal/2024-01-01.md'))
+    ).toBeFalsy();
+    expect(
+      matcher.isMatch(URI.placeholder('/workspace/notes/missing.md'))
+    ).toBeTruthy();
+  });
+
+  it('matches a wikilink placeholder on its own path', () => {
+    const matcher = new PlaceholderMatcher([
+      { uri: root, exclude: ['journal/**'] },
+    ]);
+
+    expect(matcher.isMatch(URI.placeholder('journal/2024-01-01'))).toBeFalsy();
+    // A leading slash in a wikilink means the workspace root
+    expect(matcher.isMatch(URI.placeholder('/journal/2024-01-01'))).toBeFalsy();
+    expect(matcher.isMatch(URI.placeholder('notes/missing'))).toBeTruthy();
+  });
+
+  it("applies every root's excludes to a wikilink placeholder", () => {
+    const matcher = new PlaceholderMatcher([
+      { uri: URI.file('/notes-root'), exclude: [] },
+      { uri: URI.file('/journal-root'), exclude: ['journal/**'] },
+    ]);
+
+    expect(matcher.isMatch(URI.placeholder('journal/2024-01-01'))).toBeFalsy();
+  });
+});

packages/foam-core/src/services/glob-matcher.ts

@@ -26,6 +26,23 @@ const MATCH_OPTIONS: micromatch.Options = { dot: true, nocase: true };
 
 const asPrefix = (path: string) => (path.endsWith('/') ? path : path + '/');
 
+/** With nested roots, the innermost one owns the path. */
+function findOwner<R extends { prefix: string }>(
+  roots: R[],
+  path: string
+): R | undefined {
+  let owner: R | undefined;
+  for (const root of roots) {
+    if (
+      path.startsWith(root.prefix) &&
+      (!owner || root.prefix.length > owner.prefix.length)
+    ) {
+      owner = root;
+    }
+  }
+  return owner;
+}
+
 /**
  * An {@link IMatcher} that answers from the include/exclude globs directly.
  *
@@ -55,16 +72,7 @@ export class GlobMatcher implements IMatcher {
   }
 
   isMatch(uri: URI): boolean {
-    // With nested roots, the innermost one owns the file.
-    let owner: (typeof this.roots)[number] | undefined;
-    for (const root of this.roots) {
-      if (
-        uri.path.startsWith(root.prefix) &&
-        (!owner || root.prefix.length > owner.prefix.length)
-      ) {
-        owner = root;
-      }
-    }
+    const owner = findOwner(this.roots, uri.path);
     if (!owner || owner.include.length === 0) {
       return false;
     }
@@ -79,3 +87,53 @@ export class GlobMatcher implements IMatcher {
     return Promise.resolve();
   }
 }
+
+/**
+ * An {@link IMatcher} for placeholders, which are link targets rather than
+ * files.
+ *
+ * Only exclude globs apply: the include globs already selected the notes the
+ * links come from, and say nothing about where a missing note would live. A
+ * placeholder from a path link lies under a workspace root and is tested
+ * relative to it, like a file. One from a wikilink keeps the link text as its
+ * path (`missing-note`, `journal/2024-01-01`) and lies under no root, so that
+ * path is tested as is — a leading slash meaning the workspace root — against
+ * every root's excludes.
+ */
+export class PlaceholderMatcher implements IMatcher {
+  public readonly include = ['**/*'];
+  public readonly exclude: string[];
+
+  private readonly roots: { prefix: string; exclude: string[] }[];
+
+  constructor(roots: Omit<GlobMatcherRoot, 'include'>[]) {
+    this.roots = roots.map(r => ({
+      prefix: asPrefix(r.uri.path),
+      exclude: r.exclude,
+    }));
+    this.exclude = roots.flatMap(r => r.exclude);
+  }
+
+  match(uris: URI[]): URI[] {
+    return uris.filter(u => this.isMatch(u));
+  }
+
+  isMatch(uri: URI): boolean {
+    const owner = findOwner(this.roots, uri.path);
+    return owner
+      ? !micromatch.isMatch(
+          uri.path.slice(owner.prefix.length),
+          owner.exclude,
+          MATCH_OPTIONS
+        )
+      : !micromatch.isMatch(
+          uri.path.replace(/^\//, ''),
+          this.exclude,
+          MATCH_OPTIONS
+        );
+  }
+
+  refresh(): Promise<void> {
+    return Promise.resolve();
+  }
+}

packages/foam-vscode/src/vscode/features/notes/placeholders.spec.ts

@@ -0,0 +1,70 @@
+/* @unit-ready */
+import * as vscode from 'vscode';
+import { FoamGraph, listPlaceholders } from '@foam/core';
+import {
+  createNoteFromMarkdown,
+  createTestWorkspace,
+} from '../../../test/test-utils';
+import { withModifiedFoamConfiguration } from '../../../test/test-utils-vscode';
+import { fromVsCodeUri } from '../../utils/vsc-utils';
+import { createPlaceholderMatcher } from './placeholders';
+
+/**
+ * Runs the panel's matcher over the placeholders of a note in `docs/` that
+ * links to three missing notes: one by wikilink, one next to it, and one
+ * outside `docs/`. Returns the paths of the placeholders it hides.
+ */
+const hiddenPlaceholders = async () => {
+  const root = fromVsCodeUri(vscode.workspace.workspaceFolders[0].uri);
+  const workspace = createTestWorkspace([root]).set(
+    createNoteFromMarkdown(
+      root.joinPath('docs', 'note.md').path,
+      [
+        '[[missing-note]]',
+        '[missing](missing.md)',
+        '[elsewhere](../elsewhere/missing.md)',
+      ].join('\n\n')
+    )
+  );
+  const graph = FoamGraph.fromWorkspace(workspace);
+  try {
+    const placeholders = listPlaceholders(workspace, graph).map(p => p.uri);
+    expect(placeholders).toHaveLength(3);
+    const matcher = await createPlaceholderMatcher();
+    return {
+      root,
+      hidden: placeholders
+        .filter(uri => !matcher.isMatch(uri))
+        .map(uri => uri.path),
+    };
+  } finally {
+    graph.dispose();
+    workspace.dispose();
+  }
+};
+
+describe('Placeholders panel filter', () => {
+  it('shows every placeholder when foam.files.include is restricted to a subfolder (#1702)', async () => {
+    await withModifiedFoamConfiguration(
+      'files.include',
+      ['docs/**/*.md'],
+      async () => {
+        const { hidden } = await hiddenPlaceholders();
+
+        expect(hidden).toEqual([]);
+      }
+    );
+  });
+
+  it('hides placeholders matching foam.placeholders.exclude', async () => {
+    await withModifiedFoamConfiguration(
+      'placeholders.exclude',
+      ['elsewhere/**'],
+      async () => {
+        const { root, hidden } = await hiddenPlaceholders();
+
+        expect(hidden).toEqual([root.joinPath('elsewhere', 'missing.md').path]);
+      }
+    );
+  });
+});

packages/foam-vscode/src/vscode/features/notes/placeholders.ts

@@ -1,5 +1,5 @@
 import * as vscode from 'vscode';
-import { Foam, listPlaceholders } from '@foam/core';
+import { Foam, listPlaceholders, PlaceholderMatcher } from '@foam/core';
 import {
   createMatcherAndDataStore,
   getActiveTabUri,
@@ -16,12 +16,11 @@ import {
   groupRangesByResource,
 } from '../../utils/tree-views/tree-view-utils';
 import { IMatcher } from '@foam/core';
-import { ContextMemento } from '../../utils/vsc-utils';
+import { ContextMemento, fromVsCodeUri } from '../../utils/vsc-utils';
 import { FoamGraph } from '@foam/core';
 import { URI } from '@foam/core';
 import { FoamWorkspace } from '@foam/core';
 import { FolderTreeItem } from '../../utils/tree-views/folder-tree-provider';
-import { Config } from '@foam/core';
 import { instrumentTreeView } from '../../services/telemetry';
 
 /** Retrieve the placeholders configuration */
@@ -31,20 +30,34 @@ export function getPlaceholdersConfig(): GroupedResourcesConfig {
   return { exclude };
 }
 
+/**
+ * Decides which placeholders the panel shows: `foam.placeholders.exclude`
+ * applies, `foam.files.include` doesn't — see {@link PlaceholderMatcher}.
+ */
+export async function createPlaceholderMatcher(): Promise<IMatcher> {
+  // Only used to split the excludes by workspace folder
+  const { excludePatterns } = await createMatcherAndDataStore(
+    [],
+    getPlaceholdersConfig().exclude
+  );
+  return new PlaceholderMatcher(
+    vscode.workspace.workspaceFolders.map(folder => ({
+      uri: fromVsCodeUri(folder.uri),
+      exclude: excludePatterns.get(folder.name),
+    }))
+  );
+}
+
 export default async function activate(
   context: vscode.ExtensionContext,
   foamPromise: Promise<Foam>
 ) {
   const foam = await foamPromise;
-  const { matcher } = await createMatcherAndDataStore(
-    Config.getFilesInclude(),
-    getPlaceholdersConfig().exclude
-  );
   const provider = new PlaceholderTreeView(
     context.globalState,
     foam.workspace,
     foam.graph,
-    matcher
+    await createPlaceholderMatcher()
   );
 
   const treeView = vscode.window.createTreeView('foam-vscode.placeholders', {