nrwl / precise-commits

:sparkles: Painlessly apply Prettier by only formatting lines you have modified anyway!
MIT License
471 stars 20 forks source link

Modifications not being commited #7

Open wild-lotus opened 6 years ago

wild-lotus commented 6 years ago

Hi! I just discovered the project and it is exactly what I was looking for :) I am coming from lint-staged and I was looking for this extra precision :)

I just did my first attempt to use it within the precommit hook. I staged some unformatted files and then committed them.

I guess my expectations are the intended way to work, right? I'm using precise-commits 1.0.0 and husky 0.14.3

Please let me know if you need some extra info or clarifications.

JamesHenry commented 6 years ago

Hi @CarlosGines, thanks for raising!

Currently your expectation from lint-staged is not the way precise-commits is set up to work. We are still discussing the optimal workflow when it comes to applying the formatting to git. Some feel quite strongly that the logic and version control should be separate, as sometimes "magic" commits happening behind the scenes can be undesirable.

We may end up adding a flag for the auto-commit behaviour, I will update this issue with progress.

mfru commented 6 years ago

As for me, I would appreciate an auto-commit flag as well. Thanks for your work!

stefanlemmen commented 6 years ago

I had the same question and agree an auto commit flag would be a great addition to this awesome package.

stefanlemmen commented 6 years ago

To prevent "magic" commits, an auto stage flag would also be a nice option. So it would be easier to commit the prettified changes in a next commit.

alexeagle commented 6 years ago

@vicb is working on this for angular/angular

ghost commented 6 years ago

Why even use a precommit hook when the resulting commit is not affected in any way? This is counter-intuitive.

alexeagle commented 6 years ago

I think a post-commit hook is going to work better, because when precommit runs you don't know what additional regions might be staged into the index. Any progress @vicb ?

vicb commented 6 years ago

I have been working on trying to format partial commits.

The flow is as follow: In the pre-commit hook:

  1. Stash the changes but keep the index
  2. Format the stash
  3. Commit

In the post commit hook:

  1. Unstash the changes to restore the local modifications.

Problem is that unstash in the post commit hook also revert the formatted lines. I should figure out a way to compute only the changes that are not in the index.

I can push my work when I'm back in 10days.

domkrel commented 6 years ago

@vicb any news on your side?

vicb commented 6 years ago

Sorry for the delay in answering.

My proto is available at https://github.com/vicb/angular/tree/0326-format - check the latest commit

It is based on this SO discussion

See my previous comment for a description of what works and what doesn't.

The thing to figure out is how to fix:

Problem is that unstash in the post commit hook also revert the formatted lines. I should figure out a way to compute only the changes that are not in the index.

Happy to answer any question if I can help.

JamesHenry commented 6 years ago

Hey @vicb thanks for the update! I'm also sorry for not having much availability for this recently. Tomorrow is the end of my current client engagement and I should have some hours next week to work with you on this if that would useful?

shaun-sweet commented 6 years ago

in the package.json scripts

"scripts": {
  "precise-commits": "precise-commits",
  "precommit": "npm run precise-commits && lint-staged"
},
"lint-staged": {
  "*.js": ["git add"]
}

this will do what you guys are wanting i think

alexeagle commented 6 years ago

Not quite right because that would stage everything modified. We only want to format what you staged.

On Fri, Jun 1, 2018, 12:07 PM Shaun Sweet notifications@github.com wrote:

in the package.json scripts

"precise-commits": "precise-commits", "precommit": "npm run precise-commits && git add ."

this will do what you guys are wanting. This is the same requirement lint-staged has

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nrwl/precise-commits/issues/7#issuecomment-393980658, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5I1GVqGoSshBC5WXNfnOaCfnGE9Q5ks5t4ZDwgaJpZM4R8xhm .

shaun-sweet commented 6 years ago

@alexeagle yes, i noticed that as well, i just updated my comment for a more exact solution

pechitook commented 6 years ago

@shaun-sweet actually the solution you proposed does add the whole file. I agree a flag to commit or at least re-stage what was just fixed would be useful

mvaldesdeleon commented 6 years ago

This is what would work for me: 1) Automatically re-stage the formatting changes, so that they land with the original commit. 2) Leave the formatting changes unstaged, but ERROR OUT of the githook, so that I can abort the commit and prompt the user to selectively add the reformatted chunks into his commit and try again. Right now the only way to error out is with the check-only flag, which will not perform the formatting changes at all.

In the meantime, I will have to change to a different tool, but I'd rather use this one.

benpolinsky commented 5 years ago

So, are people always committing twice? Just trying to understand the recommended workflow. Seems like it would be undesirable to always have two commits for each... commit.

"Add Foo to Bar" "Prettier cleanup"

"Add Bar to Baz" "Prettier cleanup"

You could, of course, use a vscode prettier extension but then what's the point of this library?

drazik commented 5 years ago

For those who came here because lint-staged didn't handle partially staged files, it's does it now: https://hackernoon.com/announcing-lint-staged-with-support-for-partially-staged-files-abc24a40d3ff

Ugzuzg commented 5 years ago

@drazik, cool, this seems to be working as desired:

// lint-staged.config.js
module.exports = {
  linters: {
    '*.js': ['precise-commits', 'git add'],
  },
};
rahulbhadhoriya commented 4 years ago

for future googler : see here https://github.com/nrwl/precise-commits/issues/20#issuecomment-621110837