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

cmd/shfmt: --list exit code #1093

Closed
sdavids opened this issue Sep 19, 2024 · 3 comments
Closed

cmd/shfmt: --list exit code #1093

sdavids opened this issue Sep 19, 2024 · 3 comments

Comments

@sdavids
Copy link

sdavids commented Sep 19, 2024

$ mkdir /tmp/test && cd "$_"
$ echo 'readonly  a=1' >test.sh
$ shfmt -d test.sh
diff test.sh.orig test.sh
--- test.sh.orig
+++ test.sh
@@ -1,1 +1,1 @@
-readonly  a=1
+readonly a=1
$ echo $?
1
$ shfmt -l test.sh
test.sh
$ echo $?
0

-l/--list should also return 1.

If you consider it a breaking API change you might want to introduce --set-exit-if-changed like google-java-format.

@sdavids
Copy link
Author

sdavids commented Sep 20, 2024

The use case would be:

shfmt --list $PWD >/dev/null

vs .

shfmt --diff $PWD >/dev/null

in a Git pre-commit hook.

Currently only shfmt --diff $PWD >/dev/null works.

With --list the workaround would be something like:

diff="$(shfmt --list $PWD)"
if [ -z "${diff}" ]; then
  exit 0
else
  exit 1
fi

--list is faster than --diff and produces less I/O to throw away.

@mvdan
Copy link
Owner

mvdan commented Sep 26, 2024

There is some precedent with --diff using status codes; the diff tool does so, and gofmt -d will start doing this too very soon.

There is less precedent with --list that I'm aware of, and I'm a bit worried about breaking users with the reasonable use case of listing unformatted files, but who don't want to treat the presence of them as a failure. They likely are still interested in existing failures such as invalid syntax, so ignoring the status code entirely is not a good solution.

Personally, I think the "is empty" solution is good enough. It's pretty easy to do in shell and other programming languages.

@mvdan mvdan changed the title --list exit code cmd/shfmt: --list exit code Sep 29, 2024
@mvdan
Copy link
Owner

mvdan commented Oct 19, 2024

I'm going to close this as "won't fix" for now given the reasoning I gave above.

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

No branches or pull requests

2 participants