Skip to content

Commit

Permalink
feat: Use shorter title for function tasks with many staged files (#706)
Browse files Browse the repository at this point in the history
* refactor: remove usage of `linter` and prefer `command`
* test: remove full snapshot since new git version changed the text
* improvement: create shorter titles for function tasks with many staged files

Closes #674
  • Loading branch information
iiroj authored and okonet committed Sep 26, 2019
1 parent 88d9d4f commit 1dcdb89
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 78 deletions.
33 changes: 21 additions & 12 deletions src/makeCmdTasks.js
Expand Up @@ -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)
})

Expand Down
31 changes: 12 additions & 19 deletions src/resolveTaskFn.js
Expand Up @@ -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
Expand All @@ -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)
})
}
2 changes: 1 addition & 1 deletion src/runAll.js
Expand Up @@ -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
Expand Down
34 changes: 22 additions & 12 deletions test/makeCmdTasks.spec.js
Expand Up @@ -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')
Expand All @@ -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
Expand Down Expand Up @@ -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')
})
Expand All @@ -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')
Expand All @@ -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]')
})
})
24 changes: 12 additions & 12 deletions 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(() => {
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -80,7 +80,7 @@ describe('resolveTaskFn', () => {
expect.assertions(2)
const taskFn = resolveTaskFn({
...defaultOpts,
linter: 'git add',
command: 'git add',
gitDir: '../'
})

Expand All @@ -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)
Expand All @@ -111,7 +111,7 @@ describe('resolveTaskFn', () => {
expect.assertions(2)
const taskFn = resolveTaskFn({
...defaultOpts,
linter: 'git add',
command: 'git add',
relative: true
})

Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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()
})
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions test/resolveTaskFn.unmocked.spec.js
Expand Up @@ -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
})

Expand Down
26 changes: 6 additions & 20 deletions test/runAll.unmocked.spec.js
Expand Up @@ -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)
})

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 1dcdb89

Please sign in to comment.