fix: issue #1114 (#1116)

* fix: issue #1114 Make sure that configuration files without a version and are nested in a `.vscode` directory use the current directory as the `globRoot` if it has not been defined. * fix: use only the local config when excluding files from checking.
streetsidesoftware · Apr 2, 2021 · 77ae68a · 77ae68a
1 parent df108a3
commit 77ae68a
Show file tree

Hide file tree

Showing 15 changed files with 85 additions and 25 deletions.
diff --git a/integration-tests/config/config.json b/integration-tests/config/config.json
@@ -237,6 +237,15 @@
         "**/*.{md,tex}"
       ],
       "commit": "6bfa0299bff4b7d4ae47c1f0721f5762ed2aea6b"
+    },
+    {
+      "path": "streetsidesoftware/cspell-test-cases",
+      "url": "https://github.com/streetsidesoftware/cspell-test-cases.git",
+      "args": [
+        "**/{.*/**/,.*/**/.,,.}*"
+      ],
+      "branch": "issue-1114",
+      "commit": "issue-1114"
     }
   ]
 }
diff --git a/integration-tests/snapshots/streetsidesoftware/cspell-test-cases/snapshot.txt b/integration-tests/snapshots/streetsidesoftware/cspell-test-cases/snapshot.txt
@@ -0,0 +1,7 @@
+
+Repository: streetsidesoftware/cspell-test-cases
+Url: "https://github.com/streetsidesoftware/cspell-test-cases.git"
+Args: ["**/{.*/**/,.*/**/.,,.}*"]
+Lines:
+ CSpell: Files checked: 5, Issues found: 0 in 0 files
+ exit code: 0
diff --git a/integration-tests/src/check.ts b/integration-tests/src/check.ts
@@ -52,7 +52,7 @@ async function execCheckAndUpdate(rep: Repository, update: boolean): Promise<Che
         log(color`**********************************************\n`);
         const oldCommit = rep.commit;
         try {
-            const updatedRep = mustBeDefined(await addRepository(logger, rep.url));
+            const updatedRep = mustBeDefined(await addRepository(logger, rep.url, rep.branch));
             rep = resolveArgs(updatedRep);
         } catch (e) {
             log(color`******** fail ********`);

diff --git a/integration-tests/src/config.ts b/integration-tests/src/config.ts
@@ -36,7 +36,7 @@ export function writeConfig(config: Config): void {
  * @param url - github url of the repo
  * @param commit - commit id.
  */
-export function addRepository(path: string, url: string, commit: string): Repository {
+export function addRepository(path: string, url: string, commit: string, branch: string | undefined): Repository {
     const config = readConfig();
     const entries = new Map<string, Repository>(config.repositories.map((r) => [r.path, r]));
     const existingEntry: Partial<Repository> = entries.get(path) || {};
@@ -48,6 +48,7 @@ export function addRepository(path: string, url: string, commit: string): Reposi
         url,
         args,
         commit,
+        branch,
     };
 
     entries.set(path, entry);

diff --git a/integration-tests/src/configDef.ts b/integration-tests/src/configDef.ts
@@ -5,6 +5,7 @@ export interface Config {
 export interface Repository {
     path: string;
     url: string;
+    branch: string | undefined;
     commit: string;
     args: string[];
 }
diff --git a/integration-tests/src/repositoryHelper.test.ts b/integration-tests/src/repositoryHelper.test.ts
@@ -30,6 +30,7 @@ describe('Validate repository helper', () => {
             path,
             url,
             commit: '',
+            branch: undefined,
             args: [],
         }));
     });
@@ -53,14 +54,15 @@ describe('Validate repository helper', () => {
     );
 
     test.each`
-        msg         | repo                                                         | path
-        ${'master'} | ${'https://github.com/streetsidesoftware/regexp-worker.git'} | ${'streetsidesoftware/regexp-worker'}
+        msg             | repo                                                             | path
+        ${'master'}     | ${'https://github.com/streetsidesoftware/regexp-worker.git'}     | ${'streetsidesoftware/regexp-worker'}
+        ${'issue-1114'} | ${'https://github.com/streetsidesoftware/cspell-test-cases.git'} | ${'streetsidesoftware/cspell-test-cases'}
     `(
         'addRepository $msg $repo $path',
         async ({ repo, path }: TestCase) => {
             const logger = new CaptureLogger();
-            expect(await addRepository(logger, repo)).toEqual(expect.objectContaining({ path, url: repo }));
-            expect(mockAddRepository).toHaveBeenCalledWith(path, repo, expect.any(String));
+            expect(await addRepository(logger, repo, undefined)).toEqual(expect.objectContaining({ path, url: repo }));
+            expect(mockAddRepository).toHaveBeenCalledWith(path, repo, expect.any(String), undefined);
         },
         defaultTimeout
     );

diff --git a/integration-tests/src/repositoryHelper.ts b/integration-tests/src/repositoryHelper.ts
@@ -11,12 +11,15 @@ import { Repository } from './configDef';
 
 export const repositoryDir = Path.resolve(Path.join(__dirname, '../repositories/temp'));
 
-const maxCommitDepth = 1000;
 const minCommitDepth = 10; // To handle race condition with respect to commits.
 
 const githubUrlRegexp = /^(git@github\.com:|https:\/\/github\.com\/).+$/i;
 
-export async function addRepository(logger: Logger, url: string): Promise<Repository | undefined> {
+export async function addRepository(
+    logger: Logger,
+    url: string,
+    branch: string | undefined
+): Promise<Repository | undefined> {
     if (!url || !githubUrlRegexp.test(url)) {
         return undefined;
     }
@@ -26,7 +29,7 @@ export async function addRepository(logger: Logger, url: string): Promise<Reposi
     try {
         const repoInfo = fetchRepositoryInfoForRepo(httpsUrl);
         const { path, url, commit } = await repoInfo;
-        return Config.addRepository(path, url, commit);
+        return Config.addRepository(path, url, branch || commit, branch);
     } catch (e) {
         logger.error(e);
         return undefined;
@@ -72,7 +75,7 @@ export async function checkoutRepositoryAsync(
     if (!fs.existsSync(path)) {
         try {
             const repoInfo = await fetchRepositoryInfoForRepo(url);
-            const c = await cloneRepo(logger, url, path, commit === repoInfo.commit ? minCommitDepth : maxCommitDepth);
+            const c = await cloneRepo(logger, url, path, commit === repoInfo.commit ? minCommitDepth : undefined);
             if (!c) {
                 return false;
             }
@@ -101,12 +104,15 @@ async function cloneRepo(
     path: string,
     depth: number | undefined
 ): Promise<boolean> {
-    depth = depth || 1;
-    log(`Cloning ${url}`);
+    log(`Cloning ${url} depth: ${depth || 'unlimited'}`);
     await mkdirp(Path.dirname(path));
+    const options = ['--no-checkout'];
+    if (depth) {
+        options.push(`--depth=${depth}`);
+    }
     try {
         const git = simpleGit();
-        const c = await git.clone(url, path, ['--single-branch', '--no-checkout', `--depth=${depth}`]);
+        const c = await git.clone(url, path, options);
         log(`Cloned: ${c}`);
     } catch (e) {
         error(e);

diff --git a/integration-tests/src/run.ts b/integration-tests/src/run.ts
@@ -53,14 +53,15 @@ function run(program: commander.Command) {
 
     program
         .command('add <url>')
+        .option('-b, --branch <branch>', 'Optional branch to use.')
         .description(
             'Add a repository to be checked. Example: add "https://github.com/streetsidesoftware/cspell.git". Note: to adjust the arguments, the configuration is found in `config/config.json`',
             {
                 url: 'GitHub Url',
             }
         )
-        .action(async (url: string) => {
-            await addRepository(console, url);
+        .action(async (url: string, options: { branch?: string }) => {
+            await addRepository(console, url, options.branch);
         });
 
     program.parseAsync(process.argv);

diff --git a/packages/cspell-lib/samples/packages/sample-package/.vscode/cspell.json b/packages/cspell-lib/samples/packages/sample-package/.vscode/cspell.json
@@ -0,0 +1,6 @@
+{
+    "ignorePaths": [
+        "node_modules",
+        "*.dat"
+    ]
+}
diff --git a/packages/cspell-lib/samples/packages/sample-package/README.md b/packages/cspell-lib/samples/packages/sample-package/README.md
@@ -0,0 +1,3 @@
+# Sample package
+
+This is a sample package with a local `.vscode/cspell.json` file without a version.
diff --git a/packages/cspell-lib/samples/packages/sample-package/junk.dat b/packages/cspell-lib/samples/packages/sample-package/junk.dat
@@ -0,0 +1,7 @@
+The file contains some junk data.
+
+aslkjf;a;jjvjdjdjdjjjjd
+jdkinedkekdjd
+kiinffffed
+djdkdeeeded
+daaata
diff --git a/packages/cspell-lib/src/Settings/CSpellSettingsServer.ts b/packages/cspell-lib/src/Settings/CSpellSettingsServer.ts
@@ -18,7 +18,9 @@ import { cosmiconfig, cosmiconfigSync, OptionsSync as CosmicOptionsSync, Options
 import { GlobMatcher } from 'cspell-glob';
 import { ImportError } from './ImportError';
 
-const currentSettingsFileVersion = '0.2';
+const configSettingsFileVersion0_1 = '0.1';
+const configSettingsFileVersion0_2 = '0.2';
+const currentSettingsFileVersion = configSettingsFileVersion0_2;
 
 export const sectionCSpell = 'cSpell';
 
@@ -403,7 +405,7 @@ function versionBasedMergeList<T>(
     right: T[] | undefined,
     version: CSpellSettings['version']
 ): T[] | undefined {
-    if (version === '0.1') {
+    if (version === configSettingsFileVersion0_1) {
         return takeRightOtherwiseLeft(left, right);
     }
     return mergeListUnique(left, right);
@@ -629,13 +631,19 @@ export function extractImportErrors(settings: CSpellSettings): ImportFileRefWith
 }
 
 function resolveGlobRoot(settings: CSpellSettings, pathToSettingsFile: string): string {
-    const settingsFileDir = path.dirname(pathToSettingsFile);
+    const settingsFileDirRaw = path.dirname(pathToSettingsFile);
+    const isVSCode = path.basename(settingsFileDirRaw) === '.vscode';
+    const settingsFileDir = isVSCode ? path.dirname(settingsFileDirRaw) : settingsFileDirRaw;
     const envGlobRoot = process.env[ENV_CSPELL_GLOB_ROOT];
     const cwd = envGlobRoot || process.cwd();
     const defaultGlobRoot = envGlobRoot ?? '${cwd}';
     const rawRoot =
         settings.globRoot ??
-        (settings.version === '0.1' || (envGlobRoot && !settings.version) ? defaultGlobRoot : settingsFileDir);
+        (settings.version === configSettingsFileVersion0_1 ||
+        (envGlobRoot && !settings.version) ||
+        (isVSCode && !settings.version)
+            ? defaultGlobRoot
+            : settingsFileDir);
     const globRoot = path.resolve(settingsFileDir, rawRoot.replace('${cwd}', cwd));
     return globRoot;
 }
@@ -738,7 +746,7 @@ function normalizeSettingsGlobs(
     settings: NormalizeSettingsGlobs,
     pathToSettingsFile: string
 ): NormalizeSettingsGlobsResult {
-    const { globRoot = path.dirname(pathToSettingsFile) } = settings;
+    const { globRoot } = settings;
     if (settings.ignorePaths === undefined) return {};
 
     const ignorePaths = toGlobDef(settings.ignorePaths, globRoot, pathToSettingsFile);

diff --git a/packages/cspell-lib/src/spellCheckFile.test.ts b/packages/cspell-lib/src/spellCheckFile.test.ts
@@ -47,9 +47,10 @@ describe('Validate Spell Checking Files', () => {
         ${'src/not_found.c'} | ${{}}                       | ${{}}                                         | ${{ checked: false, errors: [errNoEnt('src/not_found.c')] }}
         ${'src/sample.c'}    | ${{}}                       | ${{}}                                         | ${{ checked: true, issues: [], localConfigFilepath: s('.cspell.json'), errors: undefined }}
         ${'src/sample.c'}    | ${{}}                       | ${{ noConfigSearch: true }}                   | ${{ checked: true, localConfigFilepath: undefined, errors: undefined }}
+        ${__filename}        | ${{}}                       | ${{ noConfigSearch: true }}                   | ${{ checked: true, localConfigFilepath: undefined, errors: undefined }}
         ${'src/sample.c'}    | ${{ noConfigSearch: true }} | ${{}}                                         | ${{ checked: true, localConfigFilepath: undefined, errors: undefined }}
-        ${'src/sample.c'}    | ${{}}                       | ${{ configFile: s('../cspell.config.json') }} | ${{ checked: true, localConfigFilepath: s('../cspell.config.json'), errors: undefined }}
-        ${'src/sample.c'}    | ${{ noConfigSearch: true }} | ${{ configFile: s('../cspell.config.json') }} | ${{ checked: true, localConfigFilepath: s('../cspell.config.json'), errors: undefined }}
+        ${__filename}        | ${{}}                       | ${{ configFile: s('../cspell.config.json') }} | ${{ checked: true, localConfigFilepath: s('../cspell.config.json'), errors: undefined }}
+        ${__filename}        | ${{ noConfigSearch: true }} | ${{ configFile: s('../cspell.config.json') }} | ${{ checked: true, localConfigFilepath: s('../cspell.config.json'), errors: undefined }}
         ${'src/sample.c'}    | ${{ noConfigSearch: true }} | ${{ noConfigSearch: false }}                  | ${{ checked: true, localConfigFilepath: s('.cspell.json'), errors: undefined }}
         ${'src/sample.c'}    | ${{}}                       | ${{}}                                         | ${{ document: expect.anything(), errors: undefined }}
         ${'src/sample.c'}    | ${{}}                       | ${{ configFile: s('../cSpell.json') }}        | ${{ checked: false, localConfigFilepath: s('../cSpell.json'), errors: [eFailed(s('../cSpell.json'))] }}
@@ -148,8 +149,10 @@ describe('Validate Spell Checking Documents', () => {
         ${f('src/sample.c')}                              | ${''}           | ${{}}                          | ${{}}                                         | ${{ checked: true, issues: [], localConfigFilepath: s('.cspell.json'), errors: undefined }}
         ${f('src/sample.c')}                              | ${''}           | ${{}}                          | ${{ noConfigSearch: true }}                   | ${{ checked: true, localConfigFilepath: undefined, errors: undefined }}
         ${f('src/sample.c')}                              | ${''}           | ${{ noConfigSearch: true }}    | ${{}}                                         | ${{ checked: true, localConfigFilepath: undefined, errors: undefined }}
-        ${f('src/sample.c')}                              | ${''}           | ${{}}                          | ${{ configFile: s('../cspell.config.json') }} | ${{ checked: true, localConfigFilepath: s('../cspell.config.json'), errors: undefined }}
-        ${f('src/sample.c')}                              | ${''}           | ${{ noConfigSearch: true }}    | ${{ configFile: s('../cspell.config.json') }} | ${{ checked: true, localConfigFilepath: s('../cspell.config.json'), errors: undefined }}
+        ${f('src/sample.c')}                              | ${''}           | ${{}}                          | ${{ configFile: s('../cspell.config.json') }} | ${{ checked: false, localConfigFilepath: s('../cspell.config.json'), errors: undefined }}
+        ${f('src/sample.c')}                              | ${''}           | ${{ noConfigSearch: true }}    | ${{ configFile: s('../cspell.config.json') }} | ${{ checked: false, localConfigFilepath: s('../cspell.config.json'), errors: undefined }}
+        ${f(__filename)}                                  | ${''}           | ${{}}                          | ${{ configFile: s('../cspell.config.json') }} | ${{ checked: true, localConfigFilepath: s('../cspell.config.json'), errors: undefined }}
+        ${f(__filename)}                                  | ${''}           | ${{ noConfigSearch: true }}    | ${{ configFile: s('../cspell.config.json') }} | ${{ checked: true, localConfigFilepath: s('../cspell.config.json'), errors: undefined }}
         ${f('src/sample.c')}                              | ${''}           | ${{ noConfigSearch: true }}    | ${{ noConfigSearch: false }}                  | ${{ checked: true, localConfigFilepath: s('.cspell.json'), errors: undefined }}
         ${f('src/sample.c')}                              | ${''}           | ${{}}                          | ${{}}                                         | ${{ document: oc(d(f('src/sample.c'))), errors: undefined }}
         ${f('src/sample.c')}                              | ${''}           | ${{}}                          | ${{ configFile: s('../cSpell.json') }}        | ${{ checked: false, localConfigFilepath: s('../cSpell.json'), errors: [eFailed(s('../cSpell.json'))] }}

diff --git a/packages/cspell-lib/src/spellCheckFile.ts b/packages/cspell-lib/src/spellCheckFile.ts
@@ -13,6 +13,7 @@ import {
 import { validateText, ValidateTextOptions, ValidationIssue } from './validator';
 import * as path from 'path';
 import { combineTextAndLanguageSettings } from './Settings/TextDocumentSettings';
+import { GlobMatcher } from 'cspell-glob';
 
 export interface SpellCheckFileOptions extends ValidateTextOptions {
     /**
@@ -155,10 +156,14 @@ async function spellCheckFullDocument(
         };
     }
 
+    const matcher = new GlobMatcher(localConfig?.ignorePaths || [], { root: process.cwd(), dot: true });
+
     const config = localConfig ? mergeSettings(settings, localConfig) : settings;
     const docSettings = determineFinalDocumentSettings(document, config);
 
-    const shouldCheck = docSettings.settings.enabled ?? true;
+    const uri = URI.parse(document.uri);
+
+    const shouldCheck = !matcher.match(uri.fsPath) && (docSettings.settings.enabled ?? true);
     const { generateSuggestions, numSuggestions } = options;
     const validateOptions = { generateSuggestions, numSuggestions };
 

diff --git a/packages/cspell/src/lint.ts b/packages/cspell/src/lint.ts
@@ -75,6 +75,7 @@ export function runLint(cfg: CSpellApplicationConfiguration): Promise<RunResult>
             `Checked: ${filename}, File type: ${config.languageId}, Language: ${config.language} ... Issues: ${result.issues.length} ${elapsed}S`,
             MessageTypes.Info
         );
+        cfg.info(`Config file Used: ${spellResult.localConfigFilepath || configInfo.source}`, MessageTypes.Info);
         cfg.info(`Dictionaries Used: ${dictionaries.join(', ')}`, MessageTypes.Info);
         return result;
     }