chore: replace statusMatrix's pattern param with filter param (#1004)

billiegoose · web-flow · commit fe6d98a548fd · 2020-01-27T00:32:58.000-05:00
BREAKING CHANGE: the `pattern` param added two dependencies to the project (globrex and globalyzer) for very little benefit since we haven't ended up using globbing anywhere else in the library. The same result can be achieved via the new function parameter `filter` which lets you provide the pattern-matching logic yourself.
diff --git a/README.md b/README.md
@@ -49,7 +49,7 @@ At the time of writing, the following breaking changes are planned:
   - [x] The `fast` argument to `pull` will be removed since it will always use the `fastCheckout` implementation.
   - [ ] The `signing` function argument of `log` will be removed, and `log` will simply always return a payload. The `payload` property will be renamed to `gpgmsg` so its purpose is more obvious. (This change is to simplify the type signature of `log` so we don't need function overloading; it is the only thing blocking me from abandoning the hand-crafted `index.d.ts` file and generating the TypeScript definitions directly from the JSDoc tags that already power the website docs.)
 - [ ] Any functions that currently return `Buffer` objects will instead return `Uint8Array` so we can eventually drop the bloated Buffer browser polyfill.
-- [ ] The `pattern` and globbing options will be removed so we can drop the dependencies on `globalyzer` and `globrex`, but you'll be able to bring your own `matcher` function instead.
+- [x] The `pattern` and globbing options will be removed from `statusMatrix` so we can drop the dependencies on `globalyzer` and `globrex`, but you'll be able to bring your own `filter` function instead.
 - [ ] The `autoTranslateSSH` feature will be removed, since it's trivial to implement using just the `UnknownTransportError.data.suggestion`
 
 ## Getting Started
diff --git a/__tests__/test-statusMatrix.js b/__tests__/test-statusMatrix.js
@@ -33,17 +33,17 @@ describe('statusMatrix', () => {
     await fs.write(path.join(dir, 'a.txt'), 'Hi')
     await add({ dir, gitdir, filepath: 'a.txt' })
     await fs.write(path.join(dir, 'a.txt'), acontent)
-    matrix = await statusMatrix({ dir, gitdir, pattern: 'a.txt' })
+    matrix = await statusMatrix({ dir, gitdir, filepaths: ['a.txt'] })
     expect(matrix).toEqual([['a.txt', 1, 1, 3]])
 
     await remove({ dir, gitdir, filepath: 'a.txt' })
-    matrix = await statusMatrix({ dir, gitdir, pattern: 'a.txt' })
+    matrix = await statusMatrix({ dir, gitdir, filepaths: ['a.txt'] })
     expect(matrix).toEqual([['a.txt', 1, 1, 0]])
 
     await fs.write(path.join(dir, 'e.txt'), 'Hi')
     await add({ dir, gitdir, filepath: 'e.txt' })
     await fs.rm(path.join(dir, 'e.txt'))
-    matrix = await statusMatrix({ dir, gitdir, pattern: 'e.txt' })
+    matrix = await statusMatrix({ dir, gitdir, filepaths: ['e.txt'] })
     expect(matrix).toEqual([['e.txt', 0, 0, 3]])
   })
 
@@ -54,13 +54,13 @@ describe('statusMatrix', () => {
     await fs.write(path.join(dir, 'b.txt'), 'Hi')
     await add({ dir, gitdir, filepath: 'b.txt' })
     // Test
-    const a = await statusMatrix({ dir, gitdir, pattern: 'a.txt' })
+    const a = await statusMatrix({ dir, gitdir, filepaths: ['a.txt'] })
     expect(a).toEqual([['a.txt', 0, 2, 0]])
-    const b = await statusMatrix({ dir, gitdir, pattern: 'b.txt' })
+    const b = await statusMatrix({ dir, gitdir, filepaths: ['b.txt'] })
     expect(b).toEqual([['b.txt', 0, 2, 2]])
   })
 
-  it('statusMatrix (pattern vs filepaths)', async () => {
+  it('statusMatrix with filepaths', async () => {
     // Setup
     const { dir, gitdir } = await makeFixture('test-statusMatrix-filepath')
     // Test
@@ -76,9 +76,6 @@ describe('statusMatrix', () => {
       ['i/i.txt', 0, 2, 0]
     ])
 
-    matrix = await statusMatrix({ dir, gitdir, pattern: 'i' })
-    expect(matrix).toEqual([])
-
     matrix = await statusMatrix({ dir, gitdir, filepaths: ['i'] })
     expect(matrix).toEqual([['i/.gitignore', 0, 2, 0], ['i/i.txt', 0, 2, 0]])
 
@@ -93,54 +90,35 @@ describe('statusMatrix', () => {
     ])
   })
 
-  it('statusMatrix (pattern vs pattern + filepaths)', async () => {
+  it('statusMatrix with filter', async () => {
     // Setup
     const { dir, gitdir } = await makeFixture('test-statusMatrix-filepath')
     // Test
-    let matrix = await statusMatrix({ dir, gitdir, pattern: '*.txt' })
-    expect(matrix).toEqual([
-      ['a.txt', 1, 1, 1],
-      ['b.txt', 1, 2, 1],
-      ['c.txt', 1, 0, 1],
-      ['d.txt', 0, 2, 0]
-    ])
-
-    matrix = await statusMatrix({
+    let matrix = await statusMatrix({
       dir,
       gitdir,
-      pattern: '*.txt',
-      filepaths: ['i']
-    })
-    expect(matrix).toEqual([['i/i.txt', 0, 2, 0]])
-
-    matrix = await statusMatrix({
-      dir,
-      gitdir,
-      pattern: '*.txt',
-      filepaths: ['.', 'i']
+      filter: filepath => !filepath.includes('/') && filepath.endsWith('.txt')
     })
     expect(matrix).toEqual([
       ['a.txt', 1, 1, 1],
       ['b.txt', 1, 2, 1],
       ['c.txt', 1, 0, 1],
-      ['d.txt', 0, 2, 0],
-      ['i/i.txt', 0, 2, 0]
+      ['d.txt', 0, 2, 0]
     ])
 
     matrix = await statusMatrix({
       dir,
       gitdir,
-      pattern: 'i/*.txt',
-      filepaths: ['.', 'i']
+      filter: filepath => filepath.endsWith('.gitignore')
     })
-    expect(matrix).toEqual([['i/i.txt', 0, 2, 0]])
+    expect(matrix).toEqual([['i/.gitignore', 0, 2, 0]])
 
     matrix = await statusMatrix({
       dir,
       gitdir,
-      pattern: 'i/*.txt',
+      filter: filepath => filepath.endsWith('.txt'),
       filepaths: ['i']
     })
-    expect(matrix).toEqual([])
+    expect(matrix).toEqual([['i/i.txt', 0, 2, 0]])
   })
 })
diff --git a/__tests__/test-submodules.js b/__tests__/test-submodules.js
@@ -1,12 +1,7 @@
 /* eslint-env node, browser, jasmine */
 const { makeFixture } = require('./__helpers__/FixtureFS.js')
 
-const {
-  clone,
-  checkout,
-  listFiles,
-  commit
-} = require('isomorphic-git')
+const { clone, checkout, listFiles, commit } = require('isomorphic-git')
 
 // this is so it works with either Node local tests or Browser WAN tests
 const localhost =
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -54,8 +54,6 @@
     "crc-32": "^1.2.0",
     "diff3": "0.0.3",
     "git-apply-delta": "0.0.7",
-    "globalyzer": "^0.1.4",
-    "globrex": "^0.1.2",
     "ignore": "^5.1.4",
     "marky": "^1.2.1",
     "minimisted": "^2.0.0",
diff --git a/src/commands/statusMatrix.js b/src/commands/statusMatrix.js
@@ -1,10 +1,7 @@
 // @ts-check
-import globrex from 'globrex'
-
 import { GitIgnoreManager } from '../managers/GitIgnoreManager.js'
 import { FileSystem } from '../models/FileSystem.js'
 import { join } from '../utils/join.js'
-import { patternRoot } from '../utils/patternRoot.js'
 import { cores } from '../utils/plugins.js'
 import { worthWalking } from '../utils/worthWalking.js'
 
@@ -140,7 +137,7 @@ import { walk } from './walk.js'
  * @param {string} [args.gitdir=join(dir, '.git')] - [required] The [git directory](dir-vs-gitdir.md) path
  * @param {string} [args.ref = 'HEAD'] - Optionally specify a different commit to compare against the workdir and stage instead of the HEAD
  * @param {string[]} [args.filepaths = ['.']] - Limit the query to the given files and directories
- * @param {string} [args.pattern = null] - Filter the results to only those whose filepath matches a glob pattern. (Pattern is relative to `filepaths` if `filepaths` is provided.)
+ * @param {function(string): boolean} [args.filter] - Filter the results to only those whose filepath matches a function.
  * @param {boolean} [args.noSubmodules = false] - If true, will skip over submodules completely
  *
  * @returns {Promise<number[][]>} Resolves with a status matrix, described below.
@@ -152,21 +149,11 @@ export async function statusMatrix ({
   fs: _fs = cores.get(core).get('fs'),
   ref = 'HEAD',
   filepaths = ['.'],
-  pattern = null,
+  filter,
   noSubmodules = false
 }) {
   try {
     const fs = new FileSystem(_fs)
-    let patternPart = ''
-    let patternGlobrex
-    if (pattern) {
-      patternPart = patternRoot(pattern)
-      if (patternPart) {
-        pattern = pattern.replace(patternPart + '/', '')
-      }
-      patternGlobrex = globrex(pattern, { globstar: true, extended: true })
-    }
-    const bases = filepaths.map(filepath => join(filepath, patternPart))
     const results = await walk({
       fs,
       dir,
@@ -186,20 +173,12 @@ export async function statusMatrix ({
           }
         }
         // match against base paths
-        if (!bases.some(base => worthWalking(filepath, base))) {
+        if (!filepaths.some(base => worthWalking(filepath, base))) {
           return null
         }
         // Late filter against file names
-        if (patternGlobrex) {
-          let match = false
-          for (const base of bases) {
-            const partToMatch = filepath.replace(base + '/', '')
-            if (patternGlobrex.regex.test(partToMatch)) {
-              match = true
-              break
-            }
-          }
-          if (!match) return
+        if (filter) {
+          if (!filter(filepath)) return
         }
 
         // For now, just bail on directories
diff --git a/src/index.d.ts b/src/index.d.ts
@@ -796,7 +796,7 @@ export function statusMatrix(args: WorkDir & GitDir & {
   fs?: any;
   ref?: string;
   filepaths?: string[];
-  pattern?: string;
+  filter?: (filepath: string) => boolean;
   noSubmodules?: boolean;
 }): Promise<StatusMatrix>;
 
diff --git a/src/utils/patternRoot.js b/src/utils/patternRoot.js
