Partially staged files #30

MatthewMi11er · 2018-05-19T22:02:32Z

Am I misunderstanding what is meant by partially staged files? If I stage all my changes for a commit and one of those files requires formatting, then when I run pretty-quick --staged it errors, doesn't restaged the file and tells me it Found partially staged file. On the other hand if no staged files required formatting then it proceeds without error.

I would have not expected this behavior unless one of the changed files had remaining unstaged changes prior to running pretty-quick

The text was updated successfully, but these errors were encountered:

azz · 2018-05-20T01:45:39Z

Sounds like a bug, cc. @mpareja.

azz · 2018-05-20T02:15:50Z

Can't immediately reproduce this. Tried:

$ yarn init -y
$ yarn add -D prettier pretty-quick@1.5.0
$ git init
$ echo 'var formatted = true;' > x.js
$ echo 'var  unformatted   = true ;' > y.js
$ git add x.js y.js
$ yarn pretty-quick --staged
yarn run v1.3.2
$ /Users/azz/code/pretty-quick-issue-30/node_modules/.bin/pretty-quick --staged
🔍  Finding changed files since git revision null.
🎯  Found 2 changed files.
✍️  Fixing up y.js.
✅  Everything is awesome!
✨  Done in 0.51s.

rdiazv · 2018-05-20T19:13:14Z

I have the same issue.

marc1404 · 2018-05-20T20:02:52Z

I'm using pretty-quick@1.5.0 and I have the same problem.

error Command failed.
Exit code: 1
Command: git
Arguments: commit -m v0.0.108
Directory: ...
Output:
husky > npm run -s precommit (node v10.1.0)

🔍  Finding changed files since git revision 6c06227.
🎯  Found 1 changed file.
✍️  Fixing up package.json.
✗ Found partially staged file package.json.
✗ Partially staged files were fixed up. Please update stage before committing.

husky > pre-commit hook failed (add --no-verify to bypass)
info Visit https://yarnpkg.com/en/docs/cli/version for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

azz · 2018-05-21T12:07:30Z

Multiple reports, reverting last change in #32. Will diagnose further once the release (1.5.1) is out.

azz · 2018-05-21T12:08:36Z

@marc1404 what was the status of package.json before you committed?

azz · 2018-05-21T12:19:41Z

1.5.1 will fix the issue https://github.com/azz/pretty-quick/releases/tag/v1.5.1

azz · 2018-05-21T12:28:38Z

If anyone is able to describe reproduction steps we should be able to fix this one and re-add the feature.

marc1404 · 2018-05-21T14:32:01Z

Thanks for reacting so quickly on this issue!
I can reproduce it on v1.5.0 by running only pretty-quick (everything working) or pretty-quick --staged in the precommit-hook.

Here is my test without* the --staged flag:

$ git diff
diff --git a/layouts/default.vue b/layouts/default.vue
index 29290c1..4b73e9d 100644
--- a/layouts/default.vue
+++ b/layouts/default.vue
@@ -29,6 +29,7 @@
 import { Navbar, Sidebar, sidebarService } from '~/src/layout';

     const test = '';
+    const test2 = '';

 export default {
     name: 'DefaultLayout',
diff --git a/package.json b/package.json
index e23dd53..cf38a12 100644
--- a/package.json
+++ b/package.json
@@ -7,7 +7,7 @@
     "scripts": {
-        "precommit": "pretty-quick --staged",
+        "precommit": "pretty-quick",
$ git commit -m "test"
husky > npm run -s precommit (node v10.1.0)

🔍  Finding changed files since git revision 4d90c6c.
🎯  Found 1 changed file.
✍️  Fixing up layouts/default.vue.
✅  Everything is awesome!

Here is the output with the --staged flag:

$ git diff
diff --git a/layouts/default.vue b/layouts/default.vue
index 29290c1..4b73e9d 100644
--- a/layouts/default.vue
+++ b/layouts/default.vue
@@ -29,6 +29,7 @@
 import { Navbar, Sidebar, sidebarService } from '~/src/layout';

     const test = '';
+    const test2 = '';

 export default {
     name: 'DefaultLayout',
$ git add .
$ git commit -m "test"
husky > npm run -s precommit (node v10.1.0)

🔍  Finding changed files since git revision 50d4fcf.
🎯  Found 1 changed file.
✍️  Fixing up layouts/default.vue.
✗ Found partially staged file layouts/default.vue.
✗ Partially staged files were fixed up. Please update stage before committing.

husky > pre-commit hook failed (add --no-verify to bypass)

azz · 2018-05-21T22:09:40Z

git diff only shows unstaged files. Could you repeat the second test above but run both git diff and git diff --staged?

marc1404 · 2018-05-22T08:00:51Z

Okay, I reverted back to v1.5.0 and did the second test again (I staged all changed files):

$ git diff
$ git diff --staged
diff --git a/layouts/default.vue b/layouts/default.vue
index d493edc..e07165e 100644
--- a/layouts/default.vue
+++ b/layouts/default.vue
@@ -28,6 +28,8 @@
 <script>
 import { Navbar, Sidebar, sidebarService } from '~/src/layout';

+    const test = '';
+
 export default {
     name: 'DefaultLayout',
     components: {
diff --git a/package.json b/package.json
index a5ad677..2bbe73d 100644
--- a/package.json
+++ b/package.json
@@ -71,7 +71,7 @@
         "minimist": "1.2.0",
         "node-notifier": "5.2.1",
         "prettier": "1.12.1",
-        "pretty-quick": "1.5.1"
+        "pretty-quick": "1.5.0"
     },
     "engines": {
         "node": ">=9.0.0",
diff --git a/yarn.lock b/yarn.lock
index 3123e2d..a0ba45b 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -4491,9 +4491,9 @@ pretty-ms@3.1.0:
     parse-ms "^1.0.0"
     plur "^2.1.2"

-pretty-quick@1.5.1:
-  version "1.5.1"
-  resolved "https://registry.yarnpkg.com/pretty-quick/-/pretty-quick-1.5.1.tgz#b36bb4fe7cdb04ed5f4ba4ba530d609cb3be4ac2"
+pretty-quick@1.5.0:
+  version "1.5.0"
+  resolved "https://registry.yarnpkg.com/pretty-quick/-/pretty-quick-1.5.0.tgz#304853ece7f8cb56bec74ba3ccd978037e7f2117"
   dependencies:
     chalk "^2.3.0"
     execa "^0.8.0"
$ git commit -m "test"
husky > npm run -s precommit (node v10.1.0)

🔍  Finding changed files since git revision 018df2d.
🎯  Found 2 changed files.
✍️  Fixing up layouts/default.vue.
✗ Found partially staged file layouts/default.vue.
✗ Partially staged files were fixed up. Please update stage before committing.

husky > pre-commit hook failed (add --no-verify to bypass)

mpareja · 2018-05-22T14:09:21Z

Okay, I have a reproduction and I think I have line-of-sight on the issue. I'll keep you posted.

azz · 2018-05-22T14:11:36Z

Thanks for investigating!

The git command checking for unstaged changes should not have been including a revision.

mpareja · 2018-05-22T15:16:28Z

Okay, I pushed a PR to resolve the issue. A couple of learnings coming out of this experience:

It would be great if someone had an idea for how to easily stand-up git integration tests. Mocking out the SCM didn't help catch this one.
It looks like I was burned by not running an npm run build in between local integration testing 😢. Sorry for putting up a broken PR!

marc1404 · 2018-05-22T15:19:57Z

Maybe something like isomorphic git could be used for git integration tests?
Thanks for the PR @mpareja !

azz · 2018-05-22T22:29:51Z

Thank you @mpareja. Reason I couldn't repro is I tried from a fresh git repo, whose revision is null.

1.6.0 will re-introduce this feature.

mpareja · 2018-05-23T13:15:42Z

The new version is out. Can you give that a go @MatthewMi11er, @marc1404 and @rdiazv ?

marc1404 · 2018-05-23T13:23:21Z

@mpareja Working out perfectly so far on 1.6.0!
Thank you all :)

rdiazv · 2018-05-23T13:23:27Z

It works great! 🎉
Thanks for fixing this so quickly 👏

MatthewMi11er · 2018-05-24T01:45:08Z

1.6.0 seems to work correctly for me! Thanks for fixing that!

sultanassi95 · 2019-12-09T14:48:11Z

Removing yarn.lock and running yarn install solved my problem.

Error Message:

✖ pretty-quick --staged found some errors. Please fix them and try committing again.
🔍  Finding changed files since git revision 871be03.
🎯  Found 32 changed files.
✍️  Fixing up src/actions/index.ts.
/home/sultan/Desktop/masterworks/pplus-admin-panel/node_modules/pretty-quick/node_modules/execa/index.js:201
		throw error;
		^

Error: Command failed with exit code 128 (Unknown system error -128): git add src/actions/index.ts
  at makeError (/home/sultan/Desktop/masterworks/pplus-admin-panel/node_modules/pretty-quick/node_modules/execa/lib/error.js:59:11)
  at Function.module.exports.sync (/home/sultan/Desktop/masterworks/pplus-admin-panel/node_modules/pretty-quick/node_modules/execa/index.js:187:17)
  at runGit (/home/sultan/Desktop/masterworks/pplus-admin-panel/node_modules/pretty-quick/dist/scms/git.js:42:52)
  at Object.stageFile (/home/sultan/Desktop/masterworks/pplus-admin-panel/node_modules/pretty-quick/dist/scms/git.js:79:3)
  at onWriteFile (/home/sultan/Desktop/masterworks/pplus-admin-panel/node_modules/pretty-quick/dist/index.js:70:15)
  at _default (/home/sultan/Desktop/masterworks/pplus-admin-panel/node_modules/pretty-quick/dist/processFiles.js:46:22)
  at _default (/home/sultan/Desktop/masterworks/pplus-admin-panel/node_modules/pretty-quick/dist/index.js:58:29)
  at Object.<anonymous> (/home/sultan/Desktop/masterworks/pplus-admin-panel/node_modules/pretty-quick/bin/pretty-quick.js:12:27)
  at Module._compile (internal/modules/cjs/loader.js:778:30)
  at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
husky > pre-commit hook failed (add --no-verify to bypass)

JonathanDCohen · 2020-03-16T17:45:08Z

I'm getting this error in v2.0.1 when I have no partially staged files, just one file in staging

danielmeyer-cn · 2020-06-05T13:17:32Z

Same for me. In v2.0.1 I'm getting this error om some random files. If i change to v1.5.1 it's working.

SudoPlz · 2021-06-02T14:15:52Z

Same on 2.0.1

AllanOricil · 2021-06-04T19:04:00Z

Im facing the same problem with 2.0.1

AllanOricil · 2021-06-04T19:26:17Z

After updating to 3.1.0 the issue is gone

AntonGrekov · 2023-07-19T19:47:40Z

Same problem on 3.1.3

azz added the question Further information is requested label May 20, 2018

azz mentioned this issue May 21, 2018

Revert "feat: safer handling of partially staged files" #32

Merged

azz added bug Something isn't working and removed question Further information is requested labels May 21, 2018

mpareja added a commit to mpareja/pretty-quick that referenced this issue May 22, 2018

Fixes prettier#30 handling of partially staged files

b86e886

The git command checking for unstaged changes should not have been including a revision.

mpareja mentioned this issue May 22, 2018

feat: safer handling of partially staged files (with fix) #33

Merged

azz closed this as completed in ea58162 May 22, 2018

This was referenced May 24, 2018

Update yarn.lock christopherthielen/typedoc-plugin-single-line-tags#41

Open

Update yarn.lock christopherthielen/typedoc-plugin-single-line-tags#42

Open

Update yarn.lock christopherthielen/typedoc-plugin-internal-external#47

Closed

This was referenced Aug 7, 2018

Update yarn.lock christopherthielen/typedoc-plugin-internal-external#88

Closed

Update yarn.lock christopherthielen/typedoc-plugin-single-line-tags#124

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partially staged files #30

Partially staged files #30

MatthewMi11er commented May 19, 2018 •

edited

Loading

azz commented May 20, 2018

azz commented May 20, 2018

rdiazv commented May 20, 2018

marc1404 commented May 20, 2018

azz commented May 21, 2018

azz commented May 21, 2018

azz commented May 21, 2018

azz commented May 21, 2018

marc1404 commented May 21, 2018

azz commented May 21, 2018

marc1404 commented May 22, 2018 •

edited

Loading

mpareja commented May 22, 2018

azz commented May 22, 2018

mpareja commented May 22, 2018

marc1404 commented May 22, 2018 •

edited

Loading

azz commented May 22, 2018

mpareja commented May 23, 2018

marc1404 commented May 23, 2018

rdiazv commented May 23, 2018

MatthewMi11er commented May 24, 2018

sultanassi95 commented Dec 9, 2019

JonathanDCohen commented Mar 16, 2020

danielmeyer-cn commented Jun 5, 2020

SudoPlz commented Jun 2, 2021 •

edited

Loading

AllanOricil commented Jun 4, 2021

AllanOricil commented Jun 4, 2021

AntonGrekov commented Jul 19, 2023

Partially staged files #30

Partially staged files #30

Comments

MatthewMi11er commented May 19, 2018 • edited Loading

azz commented May 20, 2018

azz commented May 20, 2018

rdiazv commented May 20, 2018

marc1404 commented May 20, 2018

azz commented May 21, 2018

azz commented May 21, 2018

azz commented May 21, 2018

azz commented May 21, 2018

marc1404 commented May 21, 2018

azz commented May 21, 2018

marc1404 commented May 22, 2018 • edited Loading

mpareja commented May 22, 2018

azz commented May 22, 2018

mpareja commented May 22, 2018

marc1404 commented May 22, 2018 • edited Loading

azz commented May 22, 2018

mpareja commented May 23, 2018

marc1404 commented May 23, 2018

rdiazv commented May 23, 2018

MatthewMi11er commented May 24, 2018

sultanassi95 commented Dec 9, 2019

JonathanDCohen commented Mar 16, 2020

danielmeyer-cn commented Jun 5, 2020

SudoPlz commented Jun 2, 2021 • edited Loading

AllanOricil commented Jun 4, 2021

AllanOricil commented Jun 4, 2021

AntonGrekov commented Jul 19, 2023

MatthewMi11er commented May 19, 2018 •

edited

Loading

marc1404 commented May 22, 2018 •

edited

Loading

marc1404 commented May 22, 2018 •

edited

Loading

SudoPlz commented Jun 2, 2021 •

edited

Loading