Repository: okonet/lint-staged
Pull request #706 (2019-09-25 04:17:15)

Create shorter title for function tasks with many staged files
This PR makes function task titles shorter by omitting the full list of staged filenames, instead passing them a single `[file]` as the argument. This means the task still shows the expected function, but is shorter; for example `eslint --fix [file] && git add [file]`.

The mechanism of creating the title is a bit complex, since we need to first create as long an array of `[file]` strings and evaluate the function task against it. This makes sure the output is the same as for the actual file list. After this, from a single command string multiple `[file]` entries are truncated into one, so for example `ng lint --file [file] --file [file] --file [file]` turns into `ng lint --file [file]`.

I also refactored away usages of `linter` to more generic `task` and `command`.

Fixes https://github.com/okonet/lint-staged/issues/674

codecov[bot]: # [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/706?src=pr&el=h1) Report
> Merging [#706](https://codecov.io/gh/okonet/lint-staged/pull/706?src=pr&el=desc) into [master](https://codecov.io/gh/okonet/lint-staged/commit/88d9d4f63e6208035726ea56ff355ebd1591e7c7?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/okonet/lint-staged/pull/706/graphs/tree.svg?width=650&token=LSoEgsjk4O&height=150&src=pr)](https://codecov.io/gh/okonet/lint-staged/pull/706?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #706      +/-   ##
==========================================
+ Coverage   99.67%   99.67%   +<.01%     
==========================================
  Files          11       11              
  Lines         304      307       +3     
  Branches       60       63       +3     
==========================================
+ Hits          303      306       +3     
  Misses          1        1
```


| [Impacted Files](https://codecov.io/gh/okonet/lint-staged/pull/706?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/runAll.js](https://codecov.io/gh/okonet/lint-staged/pull/706/diff?src=pr&el=tree#diff-c3JjL3J1bkFsbC5qcw==) | `100% <ø> (ø)` | :arrow_up: |
| [src/makeCmdTasks.js](https://codecov.io/gh/okonet/lint-staged/pull/706/diff?src=pr&el=tree#diff-c3JjL21ha2VDbWRUYXNrcy5qcw==) | `100% <100%> (ø)` | :arrow_up: |
| [src/resolveTaskFn.js](https://codecov.io/gh/okonet/lint-staged/pull/706/diff?src=pr&el=tree#diff-c3JjL3Jlc29sdmVUYXNrRm4uanM=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/okonet/lint-staged/pull/706?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/706?src=pr&el=footer). Last update [88d9d4f...6cb19df](https://codecov.io/gh/okonet/lint-staged/pull/706?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/makeCmdTasks.js b/src/makeCmdTasks.js
index 90726c2..82dcc83 100644
--- a/src/makeCmdTasks.js
+++ b/src/makeCmdTasks.js
@@ -8,27 +8,36 @@ const debug = require('debug')('lint-staged:make-cmd-tasks')
  * Creates and returns an array of listr tasks which map to the given commands.
  *
  * @param {object} options
- * @param {Array<string|Function>|string|Function} [options.commands]
- * @param {string} [options.gitDir]
- * @param {Array<string>} [options.pathsToLint]
+ * @param {Array<string|Function>|string|Function} options.commands
+ * @param {Array<string>} options.files
+ * @param {string} options.gitDir
  * @param {Boolean} shell
  */
-module.exports = async function makeCmdTasks({ commands, gitDir, pathsToLint, shell }) {
+module.exports = async function makeCmdTasks({ commands, files, gitDir, shell }) {
   debug('Creating listr tasks for commands %o', commands)
   const commandsArray = Array.isArray(commands) ? commands : [commands]
 
   return commandsArray.reduce((tasks, command) => {
-    // linter function may return array of commands that already include `pathsToLit`
+    // command function may return array of commands that already include `stagedFiles`
     const isFn = typeof command === 'function'
-    const resolved = isFn ? command(pathsToLint) : command
-    const linters = Array.isArray(resolved) ? resolved : [resolved] // Wrap non-array linter as array
+    const resolved = isFn ? command(files) : command
+    const commands = Array.isArray(resolved) ? resolved : [resolved] // Wrap non-array command as array
 
-    linters.forEach(linter => {
-      const task = {
-        title: linter,
-        task: resolveTaskFn({ gitDir, isFn, linter, pathsToLint, shell })
-      }
+    // Function command should not be used as the task title as-is
+    // because the resolved string it might be very long
+    // Create a matching command array with [file] in place of file names
+    let mockCommands
+    if (isFn) {
+      const mockFileList = Array(commands.length).fill('[file]')
+      const resolved = command(mockFileList)
+      mockCommands = Array.isArray(resolved) ? resolved : [resolved]
+    }
 
+    commands.forEach((command, i) => {
+      // If command is a function, use the matching mock command as title,
+      // but since might include multiple [file] arguments, shorten to one
+      const title = isFn ? mockCommands[i].replace(/\[file\].*\[file\]/, '[file]') : command
+      const task = { title, task: resolveTaskFn({ gitDir, isFn, command, files, shell }) }
       tasks.push(task)
     })
 
diff --git a/src/resolveTaskFn.js b/src/resolveTaskFn.js
index a4cebdb..33eafbe 100644
--- a/src/resolveTaskFn.js
+++ b/src/resolveTaskFn.js
@@ -77,27 +77,20 @@ function makeErr(linter, result, context = {}) {
  * if the OS is Windows.
  *
  * @param {Object} options
- * @param {String} [options.gitDir] - Current git repo path
- * @param {Boolean} [options.isFn] - Whether the linter task is a function
- * @param {string} [options.linter] — Linter task
- * @param {Array<string>} [options.pathsToLint] — Filepaths to run the linter task against
+ * @param {string} options.command — Linter task
+ * @param {String} options.gitDir - Current git repo path
+ * @param {Boolean} options.isFn - Whether the linter task is a function
+ * @param {Array<string>} options.pathsToLint — Filepaths to run the linter task against
  * @param {Boolean} [options.relative] — Whether the filepaths should be relative
  * @param {Boolean} [options.shell] — Whether to skip parsing linter task for better shell support
  * @returns {function(): Promise<Array<string>>}
  */
-module.exports = function resolveTaskFn({
-  gitDir,
-  isFn,
-  linter,
-  pathsToLint,
-  relative,
-  shell = false
-}) {
+module.exports = function resolveTaskFn({ command, files, gitDir, isFn, relative, shell = false }) {
   const execaOptions = { preferLocal: true, reject: false, shell }
 
   if (relative) {
     execaOptions.cwd = process.cwd()
-  } else if (/^git(\.exe)?/i.test(linter) && gitDir !== process.cwd()) {
+  } else if (/^git(\.exe)?/i.test(command) && gitDir !== process.cwd()) {
     // Only use gitDir as CWD if we are using the git binary
     // e.g `npm` should run tasks in the actual CWD
     execaOptions.cwd = gitDir
@@ -109,20 +102,20 @@ module.exports = function resolveTaskFn({
   if (shell) {
     execaOptions.shell = true
     // If `shell`, passed command shouldn't be parsed
-    // If `linter` is a function, command already includes `pathsToLint`.
-    cmd = isFn ? linter : `${linter} ${pathsToLint.join(' ')}`
+    // If `linter` is a function, command already includes `files`.
+    cmd = isFn ? command : `${command} ${files.join(' ')}`
   } else {
-    const [parsedCmd, ...parsedArgs] = stringArgv.parseArgsStringToArgv(linter)
+    const [parsedCmd, ...parsedArgs] = stringArgv.parseArgsStringToArgv(command)
     cmd = parsedCmd
-    args = isFn ? parsedArgs : parsedArgs.concat(pathsToLint)
+    args = isFn ? parsedArgs : parsedArgs.concat(files)
   }
 
   return ctx =>
     execLinter(cmd, args, execaOptions).then(result => {
       if (result.failed || result.killed || result.signal != null) {
-        throw makeErr(linter, result, ctx)
+        throw makeErr(command, result, ctx)
       }
 
-      return successMsg(linter)
+      return successMsg(command)
     })
 }
diff --git a/src/runAll.js b/src/runAll.js
index fe17747..5f4d652 100644
--- a/src/runAll.js
+++ b/src/runAll.js
@@ -73,7 +73,7 @@ https://github.com/okonet/lint-staged#using-js-functions-to-customize-linter-com
     title: `Running tasks for ${task.pattern}`,
     task: async () =>
       new Listr(
-        await makeCmdTasks({ commands: task.commands, gitDir, shell, pathsToLint: task.fileList }),
+        await makeCmdTasks({ commands: task.commands, files: task.fileList, gitDir, shell }),
         {
           // In sub-tasks we don't want to run concurrently
           // and we want to abort on errors


Test patch:
diff --git a/test/makeCmdTasks.spec.js b/test/makeCmdTasks.spec.js
index e9919e4..284700a 100644
--- a/test/makeCmdTasks.spec.js
+++ b/test/makeCmdTasks.spec.js
@@ -9,13 +9,13 @@ describe('makeCmdTasks', () => {
   })
 
   it('should return an array', async () => {
-    const array = await makeCmdTasks({ commands: 'test', gitDir, pathsToLint: ['test.js'] })
+    const array = await makeCmdTasks({ commands: 'test', gitDir, files: ['test.js'] })
     expect(array).toBeInstanceOf(Array)
   })
 
   it('should work with a single command', async () => {
     expect.assertions(4)
-    const res = await makeCmdTasks({ commands: 'test', gitDir, pathsToLint: ['test.js'] })
+    const res = await makeCmdTasks({ commands: 'test', gitDir, files: ['test.js'] })
     expect(res.length).toBe(1)
     const [linter] = res
     expect(linter.title).toBe('test')
@@ -30,7 +30,7 @@ describe('makeCmdTasks', () => {
     const res = await makeCmdTasks({
       commands: ['test', 'test2'],
       gitDir,
-      pathsToLint: ['test.js']
+      files: ['test.js']
     })
     expect(res.length).toBe(2)
     const [linter1, linter2] = res
@@ -58,7 +58,7 @@ describe('makeCmdTasks', () => {
   })
 
   it('should work with function linter returning a string', async () => {
-    const res = await makeCmdTasks({ commands: () => 'test', gitDir, pathsToLint: ['test.js'] })
+    const res = await makeCmdTasks({ commands: () => 'test', gitDir, files: ['test.js'] })
     expect(res.length).toBe(1)
     expect(res[0].title).toEqual('test')
   })
@@ -67,7 +67,7 @@ describe('makeCmdTasks', () => {
     const res = await makeCmdTasks({
       commands: () => ['test', 'test2'],
       gitDir,
-      pathsToLint: ['test.js']
+      files: ['test.js']
     })
     expect(res.length).toBe(2)
     expect(res[0].title).toEqual('test')
@@ -78,24 +78,34 @@ describe('makeCmdTasks', () => {
     const res = await makeCmdTasks({
       commands: filenames => filenames.map(file => `test ${file}`),
       gitDir,
-      pathsToLint: ['test.js', 'test2.js']
+      files: ['test.js', 'test2.js']
     })
     expect(res.length).toBe(2)
-    expect(res[0].title).toEqual('test test.js')
-    expect(res[1].title).toEqual('test test2.js')
+    expect(res[0].title).toEqual('test [file]')
+    expect(res[1].title).toEqual('test [file]')
   })
 
   it('should work with array of mixed string and function linters', async () => {
     const res = await makeCmdTasks({
       commands: [() => 'test', 'test2', files => files.map(file => `test ${file}`)],
       gitDir,
-      pathsToLint: ['test.js', 'test2.js', 'test3.js']
+      files: ['test.js', 'test2.js', 'test3.js']
     })
     expect(res.length).toBe(5)
     expect(res[0].title).toEqual('test')
     expect(res[1].title).toEqual('test2')
-    expect(res[2].title).toEqual('test test.js')
-    expect(res[3].title).toEqual('test test2.js')
-    expect(res[4].title).toEqual('test test3.js')
+    expect(res[2].title).toEqual('test [file]')
+    expect(res[3].title).toEqual('test [file]')
+    expect(res[4].title).toEqual('test [file]')
+  })
+
+  it('should generate short names for function tasks with long file list', async () => {
+    const res = await makeCmdTasks({
+      commands: filenames => `test ${filenames.map(file => `--file ${file}`).join(' ')}`,
+      gitDir,
+      files: Array(100).fill('file.js') // 100 times `file.js`
+    })
+    expect(res.length).toBe(1)
+    expect(res[0].title).toEqual('test --file [file]')
   })
 })
diff --git a/test/resolveTaskFn.spec.js b/test/resolveTaskFn.spec.js
index 42dd417..9b56966 100644
--- a/test/resolveTaskFn.spec.js
+++ b/test/resolveTaskFn.spec.js
@@ -1,7 +1,7 @@
 import execa from 'execa'
 import resolveTaskFn from '../src/resolveTaskFn'
 
-const defaultOpts = { pathsToLint: ['test.js'] }
+const defaultOpts = { files: ['test.js'] }
 
 describe('resolveTaskFn', () => {
   beforeEach(() => {
@@ -12,7 +12,7 @@ describe('resolveTaskFn', () => {
     expect.assertions(2)
     const taskFn = resolveTaskFn({
       ...defaultOpts,
-      linter: 'node --arg=true ./myscript.js'
+      command: 'node --arg=true ./myscript.js'
     })
 
     await taskFn()
@@ -29,7 +29,7 @@ describe('resolveTaskFn', () => {
     const taskFn = resolveTaskFn({
       ...defaultOpts,
       isFn: true,
-      linter: 'node --arg=true ./myscript.js test.js'
+      command: 'node --arg=true ./myscript.js test.js'
     })
 
     await taskFn()
@@ -47,7 +47,7 @@ describe('resolveTaskFn', () => {
       ...defaultOpts,
       isFn: true,
       shell: true,
-      linter: 'node --arg=true ./myscript.js test.js'
+      command: 'node --arg=true ./myscript.js test.js'
     })
 
     await taskFn()
@@ -64,7 +64,7 @@ describe('resolveTaskFn', () => {
     const taskFn = resolveTaskFn({
       ...defaultOpts,
       shell: true,
-      linter: 'node --arg=true ./myscript.js'
+      command: 'node --arg=true ./myscript.js'
     })
 
     await taskFn()
@@ -80,7 +80,7 @@ describe('resolveTaskFn', () => {
     expect.assertions(2)
     const taskFn = resolveTaskFn({
       ...defaultOpts,
-      linter: 'git add',
+      command: 'git add',
       gitDir: '../'
     })
 
@@ -96,7 +96,7 @@ describe('resolveTaskFn', () => {
 
   it('should not pass `gitDir` as `cwd` to `execa()` if a non-git binary is called', async () => {
     expect.assertions(2)
-    const taskFn = resolveTaskFn({ ...defaultOpts, linter: 'jest', gitDir: '../' })
+    const taskFn = resolveTaskFn({ ...defaultOpts, command: 'jest', gitDir: '../' })
 
     await taskFn()
     expect(execa).toHaveBeenCalledTimes(1)
@@ -111,7 +111,7 @@ describe('resolveTaskFn', () => {
     expect.assertions(2)
     const taskFn = resolveTaskFn({
       ...defaultOpts,
-      linter: 'git add',
+      command: 'git add',
       relative: true
     })
 
@@ -135,7 +135,7 @@ describe('resolveTaskFn', () => {
       cmd: 'mock cmd'
     })
 
-    const taskFn = resolveTaskFn({ ...defaultOpts, linter: 'mock-fail-linter' })
+    const taskFn = resolveTaskFn({ ...defaultOpts, command: 'mock-fail-linter' })
     try {
       await taskFn()
     } catch (err) {
@@ -161,7 +161,7 @@ Mock error"
       cmd: 'mock cmd'
     })
 
-    const taskFn = resolveTaskFn({ ...defaultOpts, linter: 'mock-killed-linter' })
+    const taskFn = resolveTaskFn({ ...defaultOpts, command: 'mock-killed-linter' })
     try {
       await taskFn()
     } catch (err) {
@@ -177,7 +177,7 @@ Mock error"
   it('should not set hasErrors on context if no error occur', async () => {
     expect.assertions(1)
     const context = {}
-    const taskFn = resolveTaskFn({ ...defaultOpts, linter: 'jest', gitDir: '../' })
+    const taskFn = resolveTaskFn({ ...defaultOpts, command: 'jest', gitDir: '../' })
     await taskFn(context)
     expect(context.hasErrors).toBeUndefined()
   })
@@ -191,7 +191,7 @@ Mock error"
       cmd: 'mock cmd'
     })
     const context = {}
-    const taskFn = resolveTaskFn({ ...defaultOpts, linter: 'mock-fail-linter' })
+    const taskFn = resolveTaskFn({ ...defaultOpts, command: 'mock-fail-linter' })
     expect.assertions(1)
     try {
       await taskFn(context)
diff --git a/test/resolveTaskFn.unmocked.spec.js b/test/resolveTaskFn.unmocked.spec.js
index feab1bb..a368fb7 100644
--- a/test/resolveTaskFn.unmocked.spec.js
+++ b/test/resolveTaskFn.unmocked.spec.js
@@ -5,9 +5,9 @@ jest.unmock('execa')
 describe('resolveTaskFn', () => {
   it('should call execa with shell when configured so', async () => {
     const taskFn = resolveTaskFn({
-      pathsToLint: ['package.json'],
+      command: 'node -e "process.exit(1)" || echo $?',
+      files: ['package.json'],
       isFn: true,
-      linter: 'node -e "process.exit(1)" || echo $?',
       shell: true
     })
 
diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js
index 1cc222f..cacbd7d 100644
--- a/test/runAll.unmocked.spec.js
+++ b/test/runAll.unmocked.spec.js
@@ -172,16 +172,9 @@ describe('runAll', () => {
     expect(await execGit(['show', 'HEAD:test.js'])).toEqual(testJsFilePretty.replace(/\n$/, ''))
 
     // Since edit was not staged, the file is still modified
-    expect(await execGit(['status'])).toMatchInlineSnapshot(`
-"On branch master
-Changes not staged for commit:
-  (use \\"git add <file>...\\" to update what will be committed)
-  (use \\"git checkout -- <file>...\\" to discard changes in working directory)
-
-	modified:   test.js
-
-no changes added to commit (use \\"git add\\" and/or \\"git commit -a\\")"
-`)
+    const status = await execGit(['status'])
+    expect(status).toMatch('modified:   test.js')
+    expect(status).toMatch('no changes added to commit')
     expect(await readFile('test.js')).toEqual(testJsFilePretty + appended)
   })
 
@@ -210,16 +203,9 @@ no changes added to commit (use \\"git add\\" and/or \\"git commit -a\\")"
     expect(await execGit(['show', 'HEAD:test.js'])).toEqual(testJsFilePretty.replace(/\n$/, ''))
 
     // Nothing is staged
-    expect(await execGit(['status'])).toMatchInlineSnapshot(`
-"On branch master
-Changes not staged for commit:
-  (use \\"git add <file>...\\" to update what will be committed)
-  (use \\"git checkout -- <file>...\\" to discard changes in working directory)
-
-	modified:   test.js
-
-no changes added to commit (use \\"git add\\" and/or \\"git commit -a\\")"
-`)
+    const status = await execGit(['status'])
+    expect(status).toMatch('modified:   test.js')
+    expect(status).toMatch('no changes added to commit')
 
     // File is pretty, and has been edited
     expect(await readFile('test.js')).toEqual(testJsFilePretty + appended)