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

Feature request: flag for exiting 1 if changes proposed #23

Closed
Goorzhel opened this issue Mar 31, 2024 · 5 comments
Closed

Feature request: flag for exiting 1 if changes proposed #23

Goorzhel opened this issue Mar 31, 2024 · 5 comments

Comments

@Goorzhel
Copy link

Similar to:

❯ black --help | grep -A4 -- '--check'
  --check                         Don't write the files back, just return the
                                  status. Return code 0 means nothing would
                                  change. Return code 1 means some files would
                                  be reformatted. Return code 123 means there
                                  was an internal error.
@fangpenlin
Copy link
Contributor

Hi @Goorzhel, thanks for the suggestion. I think this feature won't be too hard to implement, but could you share the scenarios where this comes handy? Usually I would love to learn the actual scenarios before adding new features so that maybe there's a better way to do it.

@Goorzhel
Copy link
Author

Goorzhel commented Apr 3, 2024

I use bean-black as a pre-commit hook, and when I see it's made changes I have to abort my commit, git add them, and try again. Having the formatter exit 1 automates the "abort commit" part.

(EDIT: I realized while writing that paragraph that I should actually be using format-on-write, but I haven't set it up yet. My point about bean-black being useful as a pre-commit hook stands.)

@fangpenlin
Copy link
Contributor

I see. I assume you're running beancount-black from hooks/pre-commit directly right? Currently we have pre-commit hooks ready for use:

https://github.com/LaunchPlatform/beancount-black/blob/master/.pre-commit-hooks.yaml

One can write a .pre-commit-config.yaml with content like

repos:
  - repo: https://github.com/LaunchPlatform/beancount-black
    rev: 1.0.0
    hooks:
      - id: beancount-black

then run

pre-commit install

to make it works. It does the file change check and aborting commit for you automatically. Would you prefer to use pre-commit instead or is there any particular reason you would rather use raw git pre-commit hook?

@Goorzhel
Copy link
Author

Goorzhel commented Apr 4, 2024

I hadn't seen that, nor am I familiar with pre-commit-the-framework, but I'll try both and report here later.

And yes, I have a script in .git/hooks/pre-commit
#!/usr/bin/env -S bash -e

ag '^#!.*python' bin -l | xargs black -q --check

read -ra changed_bc_files < <(git diff --cached --name-only --diff-filter=ACM \
  | grep beancount$ | paste -sd " " )

if [ "${#changed_bc_files[@]}" -gt 0 ]; then
  bean-black -n "${changed_bc_files[@]}"
fi

@Goorzhel
Copy link
Author

Goorzhel commented Apr 4, 2024

pre-commit is pretty cool! Consider me one of today's lucky ten thousand.

❯ echo -e "; uhhhhh\n\n\n\n\n\n" > uhh.beancount
❯ git add !$ && git commit -am uhhhh
alejandra............................................(no files to check)Skipped
beancount-black..........................................................Failed
- hook id: beancount-black
- files were modified by this hook

INFO:beancount_black.main:Processing file uhh.beancount
INFO:beancount_black.formatter:Calculate column width
INFO:beancount_black.formatter:Collecting
INFO:beancount_black.main:done

black................................................(no files to check)Skipped
❯ git diff
diff --git a/uhh.beancount b/uhh.beancount
index 10d1da1..9020a8e 100644
--- a/uhh.beancount
+++ b/uhh.beancount
@@ -1,7 +1 @@
 ; uhhhhh
-
-
-
-
-
-

@Goorzhel Goorzhel closed this as not planned Won't fix, can't repro, duplicate, stale Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants