From cce9809a2ce335a3b2c3f44e4c521270b13f9d4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Wed, 30 Oct 2019 06:47:30 +0200 Subject: [PATCH] fix: prevent Listr from hiding git add warning --- lib/generateTasks.js | 1 - lib/makeCmdTasks.js | 10 +-- lib/resolveTaskFn.js | 18 +----- lib/runAll.js | 114 ++++++++++++++++++++--------------- test/resolveTaskFn.spec.js | 16 ----- test/runAll.unmocked.spec.js | 60 +++++++++--------- 6 files changed, 101 insertions(+), 118 deletions(-) diff --git a/lib/generateTasks.js b/lib/generateTasks.js index a554e91f6..4d6dcfa69 100644 --- a/lib/generateTasks.js +++ b/lib/generateTasks.js @@ -15,7 +15,6 @@ const debug = require('debug')('lint-staged:gen-tasks') * @param {boolean} [options.gitDir] - Git root directory * @param {boolean} [options.files] - Staged filepaths * @param {boolean} [options.relative] - Whether filepaths to should be relative to gitDir - * @returns {Promise} */ module.exports = function generateTasks({ config, diff --git a/lib/makeCmdTasks.js b/lib/makeCmdTasks.js index d68dea7a5..6d1482906 100644 --- a/lib/makeCmdTasks.js +++ b/lib/makeCmdTasks.js @@ -11,10 +11,9 @@ const debug = require('debug')('lint-staged:make-cmd-tasks') * @param {Array|string|Function} options.commands * @param {Array} options.files * @param {string} options.gitDir - * @param {Function} [options.logger] * @param {Boolean} shell */ -module.exports = async function makeCmdTasks({ commands, files, gitDir, logger, shell }) { +module.exports = function makeCmdTasks({ commands, files, gitDir, shell }) { debug('Creating listr tasks for commands %o', commands) const commandsArray = Array.isArray(commands) ? commands : [commands] @@ -42,8 +41,11 @@ module.exports = async function makeCmdTasks({ commands, files, gitDir, logger, title = mockCommands[i].replace(/\[file\].*\[file\]/, '[file]') } - const task = { title, task: resolveTaskFn({ command, files, gitDir, isFn, logger, shell }) } - tasks.push(task) + tasks.push({ + title, + command, + task: resolveTaskFn({ command, files, gitDir, isFn, shell }) + }) }) return tasks diff --git a/lib/resolveTaskFn.js b/lib/resolveTaskFn.js index f9be525ad..39d999629 100644 --- a/lib/resolveTaskFn.js +++ b/lib/resolveTaskFn.js @@ -62,30 +62,14 @@ function makeErr(linter, result, context = {}) { * @param {String} options.gitDir - Current git repo path * @param {Boolean} options.isFn - Whether the linter task is a function * @param {Array} options.files — Filepaths to run the linter task against - * @param {Function} [options.logger] * @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>} */ -module.exports = function resolveTaskFn({ - command, - files, - gitDir, - isFn, - logger, - relative, - shell = false -}) { +module.exports = function resolveTaskFn({ command, files, gitDir, isFn, relative, shell = false }) { const cmd = isFn ? command : `${command} ${files.join(' ')}` debug('cmd:', cmd) - if (logger && cmd.includes('git add')) { - logger.warn(` - ${symbols.warning} ${chalk.yellow( - `Detected a task using \`git add\`. Lint-staged version 10 will automatically add any task modifications to the git index, and you should remove this command.` - )}`) - } - const execaOptions = { preferLocal: true, reject: false, shell } if (relative) { execaOptions.cwd = process.cwd() diff --git a/lib/runAll.js b/lib/runAll.js index 20d9b9ad7..06915049b 100644 --- a/lib/runAll.js +++ b/lib/runAll.js @@ -68,75 +68,89 @@ module.exports = async function runAll( `) } - const tasks = generateTasks({ config, cwd, gitDir, files, relative }).map(task => ({ - title: `Running tasks for ${task.pattern}`, - task: async () => - new Listr( - await makeCmdTasks({ - commands: task.commands, - files: task.fileList, - gitDir, - logger, - shell - }), - { + const tasks = generateTasks({ config, cwd, gitDir, files, relative }) + + // lint-staged 10 will automatically add modifications to index + // Warn user when their command includes `git add` + let hasDeprecatedGitAdd = false + + const listrTasks = tasks.map(task => { + const subTasks = makeCmdTasks({ commands: task.commands, files: task.fileList, gitDir, shell }) + + if (subTasks.some(subTask => subTask.command.includes('git add'))) { + hasDeprecatedGitAdd = true + } + + return { + title: `Running tasks for ${task.pattern}`, + task: async () => + new Listr(subTasks, { // In sub-tasks we don't want to run concurrently // and we want to abort on errors dateFormat: false, concurrent: false, exitOnError: true + }), + skip: () => { + if (task.fileList.length === 0) { + return `No staged files match ${task.pattern}` } - ), - skip: () => { - if (task.fileList.length === 0) { - return `No staged files match ${task.pattern}` + return false } - return false } - })) + }) - const listrOptions = { - dateFormat: false, - renderer: (quiet && 'silent') || (debug && 'verbose') || 'update' + if (hasDeprecatedGitAdd) { + logger.warn(`${symbols.warning} ${chalk.yellow( + `Detected a task using \`git add\`. Lint-staged version 10 will automatically add any task modifications to the git index, and you should remove this command.` + )} +`) } - // If all of the configured "linters" should be skipped + // If all of the configured tasks should be skipped // avoid executing any lint-staged logic - if (tasks.every(task => task.skip())) { + if (listrTasks.every(task => task.skip())) { logger.log('No staged files match any of provided globs.') return 'No tasks to run.' } + const listrOptions = { + dateFormat: false, + renderer: (quiet && 'silent') || (debug && 'verbose') || 'update' + } + const git = new GitWorkflow(gitDir) + const runner = new Listr( + [ + { + title: 'Preparing...', + task: () => git.stashBackup() + }, + { + title: 'Running tasks...', + task: () => new Listr(listrTasks, { ...listrOptions, concurrent: true, exitOnError: false }) + }, + { + title: 'Applying modifications...', + skip: ctx => ctx.hasErrors && 'Skipped because of errors from tasks', + task: () => git.applyModifications() + }, + { + title: 'Reverting to original state...', + enabled: ctx => ctx.hasErrors, + task: () => git.restoreOriginalState() + }, + { + title: 'Cleaning up...', + task: () => git.dropBackup() + } + ], + listrOptions + ) + try { - await new Listr( - [ - { - title: 'Preparing...', - task: () => git.stashBackup() - }, - { - title: 'Running tasks...', - task: () => new Listr(tasks, { ...listrOptions, concurrent: true, exitOnError: false }) - }, - { - title: 'Applying modifications...', - skip: ctx => ctx.hasErrors && 'Skipped because of errors from tasks', - task: () => git.applyModifications() - }, - { - title: 'Reverting to original state...', - enabled: ctx => ctx.hasErrors, - task: () => git.restoreOriginalState() - }, - { - title: 'Cleaning up...', - task: () => git.dropBackup() - } - ], - listrOptions - ).run() + await runner.run() } catch (error) { if (error.message.includes('Another git process seems to be running in this repository')) { logger.error(` diff --git a/test/resolveTaskFn.spec.js b/test/resolveTaskFn.spec.js index cd038a59b..56bab7270 100644 --- a/test/resolveTaskFn.spec.js +++ b/test/resolveTaskFn.spec.js @@ -1,5 +1,4 @@ import execa from 'execa' -import makeConsoleMock from 'consolemock' import resolveTaskFn from '../lib/resolveTaskFn' @@ -201,19 +200,4 @@ describe('resolveTaskFn', () => { expect(context.hasErrors).toEqual(true) } }) - - it('should warn when tasks include git add', async () => { - const logger = makeConsoleMock() - await resolveTaskFn({ - ...defaultOpts, - command: 'git add', - logger, - relative: true - }) - expect(logger.printHistory()).toMatchInlineSnapshot(` - " - WARN - ‼ Detected a task using \`git add\`. Lint-staged version 10 will automatically add any task modifications to the git index, and you should remove this command." - `) - }) }) diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js index 044c54dee..b76e6b948 100644 --- a/test/runAll.unmocked.spec.js +++ b/test/runAll.unmocked.spec.js @@ -340,8 +340,8 @@ describe('runAll', () => { expect(error.message).toMatch('Another git process seems to be running in this repository') expect(console.printHistory()).toMatchInlineSnapshot(` " - WARN - ‼ Detected a task using \`git add\`. Lint-staged version 10 will automatically add any task modifications to the git index, and you should remove this command. + WARN ‼ Detected a task using \`git add\`. Lint-staged version 10 will automatically add any task modifications to the git index, and you should remove this command. + ERROR × lint-staged failed due to a git error. Any lost modifications can be restored from a git stash: @@ -360,17 +360,17 @@ describe('runAll', () => { // But local modifications are gone expect(await execGit(['diff'])).not.toEqual(diff) expect(await execGit(['diff'])).toMatchInlineSnapshot(` - "diff --git a/test.js b/test.js - index f80f875..1c5643c 100644 - --- a/test.js - +++ b/test.js - @@ -1,3 +1,3 @@ - module.exports = { - - 'foo': 'bar', - -} - + foo: \\"bar\\" - +};" - `) + "diff --git a/test.js b/test.js + index f80f875..1c5643c 100644 + --- a/test.js + +++ b/test.js + @@ -1,3 +1,3 @@ + module.exports = { + - 'foo': 'bar', + -} + + foo: \\"bar\\" + +};" + `) expect(await readFile('test.js')).not.toEqual(testJsFileUgly + appended) expect(await readFile('test.js')).toEqual(testJsFilePretty) @@ -424,13 +424,13 @@ describe('runAll', () => { } expect(await readFile('test.js')).toMatchInlineSnapshot(` - "<<<<<<< HEAD - module.exports = \\"foo\\"; - ======= - module.exports = \\"bar\\"; - >>>>>>> branch-b - " - `) + "<<<<<<< HEAD + module.exports = \\"foo\\"; + ======= + module.exports = \\"bar\\"; + >>>>>>> branch-b + " + `) // Fix conflict and commit using lint-staged await writeFile('test.js', fileInBranchB) @@ -444,12 +444,12 @@ describe('runAll', () => { // Nothing is wrong, so a new commit is created and file is pretty expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('4') expect(await execGit(['log', '-1', '--pretty=%B'])).toMatchInlineSnapshot(` - "Merge branch 'branch-b' + "Merge branch 'branch-b' - # Conflicts: - # test.js - " - `) + # Conflicts: + # test.js + " + `) expect(await readFile('test.js')).toEqual(fileInBranchBFixed) }) @@ -490,13 +490,13 @@ describe('runAll', () => { expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('1') expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('initial commit') expect(await readFile('README.md')).toMatchInlineSnapshot(` - "# Test + "# Test - ## Amended + ## Amended - ## Edited - " - `) + ## Edited + " + `) expect(await readFile('test-untracked.js')).toEqual(testJsFilePretty) const status = await execGit(['status']) expect(status).toMatch('modified: README.md')