Revamp findFiles

[TomasHubelbauer]

Hello, findFiles's exclude argument says that if you pass in undefined, default excludes will apply.

I have found this to not work in my extension despite my configuration being correct. Consider this piece of code:

const files = await workspace.findFiles('**/*.md'); // https://github.com/Microsoft/vscode/issues/47645
// TODO: Figure out https://github.com/Microsoft/vscode/issues/48674
console.log(workspace.getConfiguration('search.exclude'));
for (const file of files) {
    console.log(file.fsPath);
}

Above, workspace.getConfiguration('search.exclude') returns:

{
    "**/node_modules": true,
    "**/bower_components": true
  }

This is merged from the default settings, I never change this configuration section.

Despite that, the workspace.findFiles('**/*.md') (with or without undefined explicitly passed as a 2nd argument, shouldn't make a difference unless some weird parameter counting is going on) returns MarkDown files from my node_modules directory.

I do not know how to run extension debugging in a "none but this extension" mode, but I installed a published version of my extension and tried installing it alone in my Insiders instance and was able to reproduce the problem.

I am using VS Code 1.22.2 and VS Code Insiders 1.23.0-insider`.

Is there anything else I can do to debug this further?

[roblourens]

This uses files.exclude, but not search.exclude. @jrieken maybe docs should mention this?

[TomasHubelbauer]

Indeed in my case console.log(workspace.getConfiguration('files.exclude')) returns (the default):

{
    "**/.git": true,
    "**/.svn": true,
    "**/.hg": true,
    "**/CVS": true,
    "**/.DS_Store": true
  }

How can I turn the return value of workspace.getConfiguration('search.exclude') into something findFiles will accept? I came up with this:

// TODO: https://github.com/Microsoft/vscode/issues/48674
const excludes = await workspace.getConfiguration('search').get('exclude')!;
const globs = Object.keys(excludes).map(exclude => new RelativePattern(workspace.workspaceFolders![0], exclude));
const occurences: { [fsPath: string]: number; } = {};
for (const glob of globs) {
    // TODO: https://github.com/Microsoft/vscode/issues/47645
    for (const file of await workspace.findFiles('**/*.md', glob)) {
        occurences[file.fsPath] = (occurences[file.fsPath] || 0) + 1;
    }
}

// Accept only files found in results of searching of all exclude globs
const files = Object.keys(occurences).filter(fsPath => occurences[fsPath] === globs.length);

This feels hardly optimal. If there is a better way to do this, please, let me know, I'd much rather use something built-in if there's anything. I also tried merging the patterns into something like **/*.md, !**/node_modules or **/*.md; !**/node_modules, but that crashes on an invalid use of a glob ** pattern.

[roblourens]

You can combine multiple patterns into a single one by comma-separating inside {}. #38545

You can't combine include and exclude with !

And one thing to watch out for is that the exclude settings can have sibling patterns as their values, rather than true.

For example,

"files.exclude": {
	"build/**/*.js": {
		"when": "$(basename).ts"
	}
},

And the findFiles interface doesn't support those. So you would want to think about how you want to handle that.

[TomasHubelbauer]

@roblourens Can you please reopen this as a feature request for a better API for findFiles with a simple way of telling it to use excludes from getConfiguration?

[chrmarti]

This would also be helpful in our own code: b845ce9

[roblourens]

After #34711, workspaceContains patterns respect search.exclude. Quickopen does too. So I feel like findFiles should as well. But since it's a breaking change, I think it will be hard to do...

[TomasHubelbauer]

Could that be resolved by adding a new optional flag to it that would toggle this behavior on? Obviously I'd love to see it be the default behavior for this, so if it is possible to survey extension authors and assess the impact, it would be cool to take on a BC, but if that's not feasible, a flag could be an alternative.

[TomasHubelbauer]

Here's an updated snippet which will return URIs of workspace files except for those ignored either by search.exclude or files.exclude or .gitignore:

import { workspace, Uri } from "vscode";
import { exec } from 'child_process';
import applicationInsights from './telemetry';
import { join } from "path";

// TODO: https://github.com/TomasHubelbauer/vscode-extension-findFilesWithExcludes
// TODO: https://github.com/Microsoft/vscode/issues/48674 for finding MarkDown files that VS Code considers not ignored
// TODO: https://github.com/Microsoft/vscode/issues/47645 for finding MarkDown files no matter the extension (VS Code language to extension)
// TODO: https://github.com/Microsoft/vscode/issues/11838 for maybe telling if file is MarkDown using an API
// TODO: https://github.com/Microsoft/vscode/blob/release/1.27/extensions/git/src/api/git.d.ts instead of Git shell if possible
export default async function findNonIgnoredFiles(pattern: string, checkGitIgnore = true) {
  const exclude = [
    ...Object.keys(await workspace.getConfiguration('search', null).get('exclude') || {}),
    ...Object.keys(await workspace.getConfiguration('files', null).get('exclude') || {})
  ].join(',');

  const uris = await workspace.findFiles(pattern, `{${exclude}}`);
  if (!checkGitIgnore) {
    return uris;
  }

  const workspaceRelativePaths = uris.map(uri => workspace.asRelativePath(uri, false));
  for (const workspaceDirectory of workspace.workspaceFolders!) {
    const workspaceDirectoryPath = workspaceDirectory.uri.fsPath;
    try {
      const { stdout, stderr } = await new Promise<{ stdout: string, stderr: string }>((resolve, reject) => {
        exec(
          `git check-ignore ${workspaceRelativePaths.join(' ')}`,
          { cwd: workspaceDirectoryPath },
          // https://git-scm.com/docs/git-check-ignore#_exit_status
          (error: Error & { code?: 0 | 1 | 128 }, stdout, stderr) => {
            if (error && (error.code !== 0 && error.code !== 1)) {
              reject(error);
              return;
            }

            resolve({ stdout, stderr });
          },
        );
      });

      if (stderr) {
        throw new Error(stderr);
      }

      for (const relativePath of stdout.split('\n')) {
        const uri = Uri.file(join(workspaceDirectoryPath, relativePath.slice(1, -1) /* Remove quotes */));
        const index = uris.findIndex(u => u.fsPath === uri.fsPath);
        if (index > -1) {
          uris.splice(index, 1);
        }
      }
    } catch (error) {
      applicationInsights.sendTelemetryEvent('findNonIgnoredFiles-git-exec-error');
    }
  }

  return uris;
}

I would much rather VS Code extension API provided this out of the box, but alas. I am noticing some new development like FileSearchProvider which natively allows ignoring Git ignored files, but it doesn't seem like this will trickle down to the good ol' findFiles anytime soon.

Thus the code above should be the best we have for now unless there is some way to use the Git extension API to query the ignored-or-not status of a URI without using exec.

@joaomoreno is this API usable from other extensions or is that something internal to only VS Code? If it is, can you give an example of how an extension author could make use of it? I wasn't able to find anything on the topic.