search

pre-commit-ci / issues

public issues for https://pre-commit.ci
17 stars 3 forks source link

Feature request: display diff of not-applied changes #65

Open StanczakDominik opened 3 years ago

StanczakDominik commented 3 years ago

The recently added autofix_prs flag is awesome, it allowed me to convince folks at PlasmaPy to switch to pre-commit.ci! However, one thing that's a bit annoying as a result is that, if a pre-commit hook fails, the actual applied changes are nowhere to be seen:

image

With autofix_prs disabled, this means one has to either run pre-commit autofix (which modifies the branch, and people can be sensitive about touching their branches) or install pre-commit locally.

But the pre-commit result would already be there, and showing the diff would probably be a simple matter of adding --show-diff-on-failure (possibly optionally, if not autofix_prs?) to pre-commit run, if I'm correct?

asottile commented 3 years ago

the diff in most cases is too large to store / show without drastically increasing the operating costs (as even with 2 files of diff it's going to be bigger than just the pre-commit output)

asottile commented 3 years ago

(I could show it, but 99% of the time it's going to get cut off by the output limiter)

asottile commented 3 years ago

hmmm one idea would be to push anyway, but to an orphaned commit (which github keeps around forever) and provide a link -- let me see how difficult that would be

StanczakDominik commented 3 years ago

the diff in most cases is too large to store / show without drastically increasing the operating costs (as even with 2 files of diff it's going to be bigger than just the pre-commit output)

Oh crap, I didn't think of that. In that case, if the push-to-orphan commit idea doesn't end up feasible, feel free to close this one!

StanczakDominik commented 3 years ago

Hey, out of curiosity, did you ever try out the push-to-orphan idea?

asottile commented 3 years ago

it would work but it's not easy so I haven't had a chance to implement it yet (I need to change the push code to use the api instead of git)

On Thu, Aug 12, 2021, 11:27 AM Dominik Stańczak @.***> wrote:

Hey, out of curiosity, did you ever try out the push-to-orphan idea?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pre-commit-ci/issues/issues/65#issuecomment-897734429, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN2BH6VF72JGLM7NVLT6CTT4PR5JANCNFSM44NNPJVQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .