tweag / cooked-validators

MIT License
39 stars 11 forks source link

Pre commit git hook interferes with partially-staged files #188

Closed florentc closed 1 year ago

florentc commented 1 year ago

We provide a pre-commit hook to automatically format source files with ormolu in ci/ormolu-pre-commit-hook.sh

The script adds (git add) the source files. When trying to only stage parts of a modified file for a commit (using git add -p for example), this hook overrides it and stages all the changes in the file, making it impossible to only commit some parts.

I don't think there is an easy way to sort this out. In theory we should be able to identify staged hunks and only apply ormolu on those lines, then stage these hunks again, but this seems far from trivial and this would be extremely low priority work.

Alternatives would be:

gabrielhdt commented 1 year ago

The following hook would just prevent people from committing if some file is not properly formatted

#!/bin/sh
ormolu --mode check $(find . -name '*.hs')
Niols commented 1 year ago

In general, I am very much against commit hooks that do anything else that accepting/rejecting commits, so I'd be more for a solutoin like @gabrielhdt's. However, I would appreciate more granularity and I'd like to avoid my commits being rejected because of a file that is unstaged (or even ignored!). I have been using one quite successfully before, I'll find it back for us and we can see if you like it. Its only downside is that it rejects commits that stage only part of a file but where the unstaged part is badly formatted.

gabrielhdt commented 1 year ago

Maybe something like

#!/bin/sh
ormolu --mode check $(git diff --name-only --cached | grep '\.hs$')

which should only list files in the next commit

Niols commented 1 year ago

I remember trying something like this and being unsatisfied. I think it behaves poorly with added or deleted files? I ended up using git status --porcelain followed by sed for filtering and processing.