From ea58162d4f2462305ea8b0859eb70325c788a418 Mon Sep 17 00:00:00 2001 From: Mario Pareja Date: Tue, 22 May 2018 18:28:22 -0400 Subject: [PATCH] feat: safer handling of partially staged files (with fix) (#33) * feat: safer handling of partially staged files (#29) + Partially staged files are not re-staged + Non-zero exit code upon reformatting partially staged file + Update README * Fixes #30 handling of partially staged files The git command checking for unstaged changes should not have been including a revision. --- README.md | 2 ++ bin/pretty-quick.js | 16 +++++++++++++- src/__tests__/scm-git.test.js | 39 ++++++++++++++++++++++++++++++----- src/index.js | 18 +++++++++++++++- src/scms/git.js | 4 ++++ src/scms/hg.js | 4 ++++ 6 files changed, 76 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index ee8c710..66332e9 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,8 @@ In `package.json`'s `"scripts"` section, add: Pre-commit mode. Under this flag only staged files will be formatted, and they will be re-staged after formatting. +Partially staged files will not be re-staged after formatting and pretty-quick will exit with a non-zero exit code. The intent is to abort the git commit and allow the user to amend their selective staging to include formatting fixes. + ### `--branch` When not in `staged` pre-commit mode, use this flag to compare changes with the specified branch. Defaults to `master` (git) / `default` (hg) branch. diff --git a/bin/pretty-quick.js b/bin/pretty-quick.js index 373f07c..b5885d5 100755 --- a/bin/pretty-quick.js +++ b/bin/pretty-quick.js @@ -9,6 +9,7 @@ const prettyQuick = require('..').default; const args = mri(process.argv.slice(2)); +let success = true; prettyQuick( process.cwd(), Object.assign({}, args, { @@ -28,10 +29,23 @@ prettyQuick( ); }, + onPartiallyStagedFile: file => { + console.log(`✗ Found ${chalk.bold('partially')} staged file ${file}.`); + success = false; + }, + onWriteFile: file => { console.log(`✍️ Fixing up ${chalk.bold(file)}.`); }, }) ); -console.log('✅ Everything is awesome!'); +if (success) { + console.log('✅ Everything is awesome!'); +} else { + console.log( + '✗ Partially staged files were fixed up.' + + ` ${chalk.bold('Please update stage before committing')}.` + ); + process.exit(1); // ensure git hooks abort +} diff --git a/src/__tests__/scm-git.test.js b/src/__tests__/scm-git.test.js index 6af4369..d0c6f6e 100644 --- a/src/__tests__/scm-git.test.js +++ b/src/__tests__/scm-git.test.js @@ -11,9 +11,10 @@ afterEach(() => { jest.clearAllMocks(); }); -const mockGitFs = () => { +const mockGitFs = (additionalUnstaged = '') => { mock({ '/.git': {}, + '/raz.js': 'raz()', '/foo.js': 'foo()', '/bar.md': '# foo', }); @@ -26,8 +27,8 @@ const mockGitFs = () => { return { stdout: '' }; case 'diff': return args[2] === '--cached' - ? { stdout: './foo.js\n' } - : { stdout: './foo.js\n' + './bar.md\n' }; + ? { stdout: './raz.js\n' } + : { stdout: './foo.js\n' + './bar.md\n' + additionalUnstaged }; case 'add': return { stdout: '' }; default: @@ -80,6 +81,20 @@ describe('with git', () => { ]); }); + test('with --staged calls diff without revision', () => { + mock({ + '/.git': {}, + }); + + prettyQuick('root', { since: 'banana', staged: true }); + + expect(execa.sync).toHaveBeenCalledWith( + 'git', + ['diff', '--name-only', '--diff-filter=ACMRTUB'], + { cwd: '/' } + ); + }); + test('calls `git diff --name-only` with revision', () => { mock({ '/.git': {}, @@ -151,12 +166,15 @@ describe('with git', () => { expect(fs.readFileSync('/bar.md', 'utf8')).toEqual('formatted:# foo'); }); - test('with --staged stages staged files', () => { + test('with --staged stages fully-staged files', () => { mockGitFs(); prettyQuick('root', { since: 'banana', staged: true }); - expect(execa.sync).toHaveBeenCalledWith('git', ['add', './foo.js'], { + expect(execa.sync).toHaveBeenCalledWith('git', ['add', './raz.js'], { + cwd: '/', + }); + expect(execa.sync).not.toHaveBeenCalledWith('git', ['add', './foo.md'], { cwd: '/', }); expect(execa.sync).not.toHaveBeenCalledWith('git', ['add', './bar.md'], { @@ -164,6 +182,17 @@ describe('with git', () => { }); }); + test('with --staged does not stage previously partially staged files AND aborts commit', () => { + const additionalUnstaged = './raz.js\n'; // raz.js is partly staged and partly not staged + mockGitFs(additionalUnstaged); + + prettyQuick('root', { since: 'banana', staged: true }); + + expect(execa.sync).not.toHaveBeenCalledWith('git', ['add', './raz.js'], { + cwd: '/', + }); + }); + test('without --staged does NOT stage changed files', () => { mockGitFs(); diff --git a/src/index.js b/src/index.js index 4c2fd41..5076ca0 100644 --- a/src/index.js +++ b/src/index.js @@ -12,6 +12,7 @@ export default ( branch, onFoundSinceRevision, onFoundChangedFiles, + onPartiallyStagedFile, onWriteFile, } = {} ) => { @@ -30,13 +31,28 @@ export default ( .filter(isSupportedExtension) .filter(createIgnorer(directory)); + const unstagedFiles = staged + ? scm + .getUnstagedChangedFiles(directory, revision) + .filter(isSupportedExtension) + .filter(createIgnorer(directory)) + : []; + + const wasFullyStaged = f => unstagedFiles.indexOf(f) < 0; + onFoundChangedFiles && onFoundChangedFiles(changedFiles); formatFiles(directory, changedFiles, { config, onWriteFile: file => { onWriteFile && onWriteFile(file); - staged && scm.stageFile(directory, file); + if (staged) { + if (wasFullyStaged(file)) { + scm.stageFile(directory, file); + } else { + onPartiallyStagedFile && onPartiallyStagedFile(file); + } + } }, }); }; diff --git a/src/scms/git.js b/src/scms/git.js index c6cc814..8fa1dfb 100644 --- a/src/scms/git.js +++ b/src/scms/git.js @@ -61,6 +61,10 @@ export const getChangedFiles = (directory, revision, staged) => { ].filter(Boolean); }; +export const getUnstagedChangedFiles = directory => { + return getChangedFiles(directory, null, false); +}; + export const stageFile = (directory, file) => { runGit(directory, ['add', file]); }; diff --git a/src/scms/hg.js b/src/scms/hg.js index 37a074c..1cf312b 100644 --- a/src/scms/hg.js +++ b/src/scms/hg.js @@ -35,6 +35,10 @@ export const getChangedFiles = (directory, revision) => { ].filter(Boolean); }; +export const getUnstagedChangedFiles = () => { + return []; +}; + export const stageFile = (directory, file) => { runHg(directory, ['add', file]); };