lspitzner / brittany

haskell source code formatter
GNU Affero General Public License v3.0
690 stars 72 forks source link

Support `check` mode for CI #248

Closed NobbZ closed 5 years ago

NobbZ commented 5 years ago

Many formatters across have some check mode, that would return a non-zero exit code when any of the provided files would need a reformatting.

Is there something similar available for brittany?

A big plus if it would print the path to those files.

mbj commented 5 years ago

I cannot answer OP but:

A workaround: Run the formatter in fix mode, and fail if the git repository is dirty afterwards.

NobbZ commented 5 years ago

Which does work for CI, but not for a pre-commit-hook…

ElvishJerricco commented 5 years ago

Isn't this exactly what --check-mode does?

NobbZ commented 5 years ago
$ brittany --check-mode
brittany: error parsing arguments: could not parse input/unprocessed input
$ brittany --version
brittany version 0.11.0.0

Installed via stack

NobbZ commented 5 years ago

Okay… Just found that in fact with 0.12 --check-mode is available…

Perhaps try to get it into stackage LTS?

tfausak commented 5 years ago

Brittany 0.12 is in Stackage LTS 14: https://www.stackage.org/lts-14.1/package/brittany-0.12.0.0

I had forgotten that this feature was added. Since there's a note in the change log and the flag shows up in --help, I'm going to close this issue.

andys8 commented 4 years ago

I'm currently using brittany --config-file brittany.yaml --check-mode **/*.hs on CI. I can't figure out which argument to pass to print the cause why the check failed. It currently exits with 1 but doesn't print anything. Does somebody know how to configure this or is it currently not possible?

lspitzner commented 4 years ago

@andys8

Currently not possible, but it would be trivial to at least print the file names where formatting would change anything.

Or do we want more? I just thought "oh, diffs might be neat too" but this feels like yet another mode. My inner perfectionist now wants to switch the different boolean flags into a single --mode=(check|diff|output|replace), but it is probably not worth a breaking change.

I should probably just implement the print-paths-of-files-that-would-change, but one more question: should we suppress warnings in those cases? otherwise you'd get a mix of warnings and files names on stderr. Or maybe the file paths should go to stdout and warnings/errors to stderr? Would that work for you?

andys8 commented 4 years ago

Since I wouldn't be processing the output but/log print them on CI, a mix of warnings and file names wouldn't be an issue. I'm not aware of others, though. I think diffs would be nice, but file names only would work, too.

The use case is that CI should make sure team members are using the same formatter with the same config and the same version. Without a hint it would be hard to have any clue why there is a difference.

Thanks for tackling the issue :)

lspitzner commented 4 years ago

Implemented printing to stdout in 85d55c376839abf119b821c687b03665ed8a8f60.

andys8 commented 4 years ago

Nice 🤩