Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --staged flag to only check changes in staged code #4328

Open
abemedia opened this issue Jan 21, 2024 · 7 comments
Open

Add --staged flag to only check changes in staged code #4328

abemedia opened this issue Jan 21, 2024 · 7 comments
Labels
enhancement New feature or improvement no decision No decision to fix or not

Comments

@abemedia
Copy link
Contributor

Your feature request related to a problem? Please describe.

I use golangci-lint in pre-commit hooks and without adding a custom script to first stash unstaged changes this results in the pre-commit hook failing if there are issues in unstaged code.

Describe the solution you'd like.

A new flag --staged which ignores all issues that are not in staged changes.

Describe alternatives you've considered.

A custom pre-commit script which first stashes the changes e.g.

git stash --keep-index --include-untracked --quiet
exitCode=0
golangci-lint run || exitCode=$?
git stash pop --quiet
exit $exitCode

The problem with this solution is that it can result in the loss of what is currently staged and what isn't.

Additional context.

No response

@abemedia abemedia added the enhancement New feature or improvement label Jan 21, 2024
Copy link

boring-cyborg bot commented Jan 21, 2024

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

@ldez ldez added the no decision No decision to fix or not label Jan 21, 2024
@ttlins
Copy link

ttlins commented Feb 13, 2024

hey @abemedia , are there any current proposals on how to tackle that? I'm looking for this exact solution, would be glad to work on PR later if you haven't started yet 👍

@abemedia
Copy link
Contributor Author

Well there's already logic to just use a patch file. One could implement this by generating a patch file from the currently staged changes.

@ldez
Copy link
Member

ldez commented Feb 13, 2024

Hello,

FYI, the current behavior is based on git ls-files --others --exclude-standard.

ls-files doesn't have an option to list only staged files (--stage just displays files with staged contents' object information, but it can be a staged file or just modified existing files)
ls-files just shows tracked/untracked files.
https://git-scm.com/docs/git-ls-files

Maybe git diff --name-only --cached can be an approach.
https://git-scm.com/docs/git-diff

@ttlins
Copy link

ttlins commented Feb 13, 2024

ah, I wasn't aware of the new-from-patch flag. That's easier than I thought then 🙌
yeah, I'd suggest using the git diff ... approach also. I'll see if I can work on that later today/tomorrow then 👍
Thanks!

@ldez
Copy link
Member

ldez commented Feb 13, 2024

My comment was not really a suggestion but more a technical thinking.

I'm not sure of the value of an option, only for a niche, that can be done easily with the command line.
This is why I flagged this issue with no-decision.

git diff --cached > /tmp/stage.patch; golangci-lint run --new-from-patch=/tmp/stage.patch; rm /tmp/stage.patch
git diff --cached > /tmp/stage.patch
golangci-lint run --new-from-patch=/tmp/stage.patch
rm /tmp/stage.patch

Maybe this can be a new hook inside our pre-commit hook file. 🤔

Also, note that running golangci-lint only on a few files can lead to unexpected behavior with linters (the majority) that require scanning all files.

@ttlins
Copy link

ttlins commented Feb 13, 2024

I'm not sure of the value of an option, only for a niche, that can be done easily with the command line.

Yeah, to be honest I've had the same thought. After reading your comments, I re-wrote my pre commit hook to do pretty much the same as you've suggested above.

I'm just sad since this was an easy first contribution 😅

Also, note that running golangci-lint only on a few files can lead to unexpected behavior with linters (the majority) that require scanning all files.

I wasn't aware of this though - thanks for raising it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement no decision No decision to fix or not
Projects
None yet
Development

No branches or pull requests

3 participants