7 ms·

Shfmt – format shell programs

115 points by typical182 4y ago

ufo 4y ago
Are there examples of what the formatted code looks like?
- mvdan 4y ago
  https://github.com/mvdan/sh/blob/master/syntax/canonical.sh https://github.com/mvdan/sh/blob/master/syntax/canonical.sh is one small example.
- usr1106 4y ago
  We introduced it at work after the code base containing had been developed for 3 - 4 years with just manual reviews. The changes we needed to make were rather small and most of them were oversights, the code should not have looked like that in the first place. Of course your mileage might vary. The only thing that disturbed me personally is no space before semicolon. Without having read what spec says I think the semicolon in shell is more of a command separator than a terminator. So I had always formatted it symmetrically, with space before and after like e.g. && or a pipe. shfmt did not support that so I had to adapt. Still having a tool and skipping review discussions outweighs this minor matter of taste.
  - erik_seaberg 4y ago
    I try to use line breaks instead, it only took a little getting used to for i in $(seq 100) do if expr $i % 5 > /dev/null then if expr $i % 3 > /dev/null then echo $i else echo fizz fi else if expr $i % 3 > /dev/null then echo buzz else echo fizzbuzz fi fi done
    - usr1106 4y ago
      I did that at university around 30 years ago. I think nowadays 1TBS is much more common.
      - erik_seaberg 4y ago
        Yeah, I use 1TBS and cuddled elses in Scala/Java/C++, but adding a semicolon to do it with a keyword feels like arguing with the grammar.
typical182 4y ago
shfmt is like gofmt, rustfmt, ..., but for shell programs. Supports bash, posix, mksh, bats.
deleted 4y ago
[deleted]
VWWHFSfQ 4y ago
The caveats are somewhat of a deal breaker unfortunately. It fails on perfectly valid syntax because they don't want to complicate the parser. But that makes the tool not very useful.
- Spivak 4y ago
  I can’t say I’ve ever used the forms they don’t support and I am the person who uses all sorts of esoteric bash features at work. Also for static typing an analysis I would absolutely give up even more syntax that is ambiguously parsed.
  - jamal-kumar 4y ago
    I throw all my shell scripts through this beast of a haskell application to see if they're clean: https://github.com/koalaman/shellcheck https://github.com/koalaman/shellcheck Crucially it shows where on the line the error is in case I've got some large piped one-liner which might have a problem.
- 2h 4y ago
  Have to agree with the other response. Demanding perfection is not realistic in many situations. If you can get 95% of the way there with 50% of the code/effort, you should do it. Sometimes being productive is knowing when that last percentage just isn't worth it.
- mvdan 4y ago
  For the first two caveats, I actually agree that we could and should handle ambiguous input. It just hasn't been a priority because doing that properly would be quite a bit of work, and such ambiguous syntax isn't particularly common. See https://github.com/mvdan/sh/issues/686 https://github.com/mvdan/sh/issues/686 for my current thoughts on how to tackle it. The third caveat concerns parsing `export` and `let` as keywords rather than as builtins. Like the README says, this is to properly build the syntax tree without leaving opaque strings as expressions, but also to support `declare foo=(bar)` which wouldn't work if `declare` was treated like any other builtin simple command. How else would you have a static parser handle these two builtins? They are in a bit of an awkward middle ground between builtin and keyword. My instinct is that giving them special treatment in the parser to allow tokens like `(`, while at the same time representing them in the syntax tree with opaque strings as expressions, would be pretty underwhelming to any users of the parser. That said, we already have that problem with `let "foo=123"` for example, where our parser currently represents the expression as the quoted string without going any deeper. https://github.com/mvdan/sh/issues/754#issuecomment-963295749 https://github.com/mvdan/sh/issues/754#issuecomment-96329574... considers doing a second parse stage in the shell interpreter to fix cases like these, though always doing a second parse could get expensive. We _could_ leave all arithmetic expressions as input strings in the parser, and do all the actual parsing when they are evaluated. That would be more compatible with Bash and more consistent. But it would also be less useful to any parser users who don't run into any of these weird edge cases, which aren't common at all, I think. In short, I have some ideas, but I'm not sure at all what's best :) Doing a good job for 99% of users feels better than aiming for 100% compatibility with bash syntax, particularly where bash syntax is a bit weird.
jesusofnazarath 4y ago
[dead]
oweiler 4y ago
I've used it on a small project and it worked beautifully. It even understands and respects Bats syntax.
eschneider 4y ago
Oooh, this looks fun.
jeremy_wiebe 4y ago
And if you use fish, it comes with a built-in formatting function. https://fishshell.com/docs/current/cmds/fish_indent.html https://fishshell.com/docs/current/cmds/fish_indent.html
jmholla 4y ago
Anybody have any luck building the image? It fails for me on podman and docker.
js2 4y ago
I use shfmt and shellcheck together with pre-commit. I like to use the shfmt-py and shellcheck-py pre-commit hooks as opposed to https://github.com/jumanjihouse/pre-commit-hooks https://github.com/jumanjihouse/pre-commit-hooks as they'll install the shfmt/shellcheck prebuilt binaries as needed: https://github.com/maxwinterstein/shfmt-py https://github.com/maxwinterstein/shfmt-py https://github.com/shellcheck-py/shellcheck-py https://github.com/shellcheck-py/shellcheck-py
- one-punch 4y ago
  An alternative to have both `shfmt` and `shellcheck` and all pre-commit hooks managed (that is, automatically installed until no longer used and garbage collected) is to use https://devenv.sh/ https://devenv.sh/. After `devenv init`, update `devenv.nix` as follows: { pkgs, ... }: { pre-commit.hooks = { shellcheck.enable = true; shfmt.enable = true; }; }
moondev 4y ago
Another gem from the same repo - gosh - pure golang shell This means anywhere golang is installed, including aarch64 Darwin and Windows you can: go run mvdan.cc/sh/v3/cmd/gosh@latest Or things like go run mvdan.cc/sh/v3/cmd/gosh@latest -c 'echo "cross platform shell"; go run github.com/mikefarah/yq/v3@latest r metadata.name <(kubectl get pod my-pod -o yaml)' Pretty awesome stuff, I'm always discovering new ways to use it.
- akshayshah 4y ago
  At a previous job, my team slowly built an absolutely unmanageable bash script (hundreds of lines, including _many_ complex, multi-line jq incantations). The first step in migrating that script to go was to embed it in a Go binary and run it with gosh. Kudos to Daniel for building such a wonderful package.

115 points by typical182 4y ago

ufo 4y ago

Are there examples of what the formatted code looks like?

mvdan 4y ago
https://github.com/mvdan/sh/blob/master/syntax/canonical.sh https://github.com/mvdan/sh/blob/master/syntax/canonical.sh is one small example.
usr1106 4y ago
We introduced it at work after the code base containing had been developed for 3 - 4 years with just manual reviews. The changes we needed to make were rather small and most of them were oversights, the code should not have looked like that in the first place. Of course your mileage might vary. The only thing that disturbed me personally is no space before semicolon. Without having read what spec says I think the semicolon in shell is more of a command separator than a terminator. So I had always formatted it symmetrically, with space before and after like e.g. && or a pipe. shfmt did not support that so I had to adapt. Still having a tool and skipping review discussions outweighs this minor matter of taste.
- erik_seaberg 4y ago
  I try to use line breaks instead, it only took a little getting used to for i in $(seq 100) do if expr $i % 5 > /dev/null then if expr $i % 3 > /dev/null then echo $i else echo fizz fi else if expr $i % 3 > /dev/null then echo buzz else echo fizzbuzz fi fi done
  - usr1106 4y ago
    I did that at university around 30 years ago. I think nowadays 1TBS is much more common.
    - erik_seaberg 4y ago
      Yeah, I use 1TBS and cuddled elses in Scala/Java/C++, but adding a semicolon to do it with a keyword feels like arguing with the grammar.

typical182 4y ago

shfmt is like gofmt, rustfmt, ..., but for shell programs. Supports bash, posix, mksh, bats.

deleted 4y ago

[deleted]

VWWHFSfQ 4y ago

The caveats are somewhat of a deal breaker unfortunately. It fails on perfectly valid syntax because they don't want to complicate the parser. But that makes the tool not very useful.

Spivak 4y ago
I can’t say I’ve ever used the forms they don’t support and I am the person who uses all sorts of esoteric bash features at work. Also for static typing an analysis I would absolutely give up even more syntax that is ambiguously parsed.
- jamal-kumar 4y ago
  I throw all my shell scripts through this beast of a haskell application to see if they're clean: https://github.com/koalaman/shellcheck https://github.com/koalaman/shellcheck Crucially it shows where on the line the error is in case I've got some large piped one-liner which might have a problem.
2h 4y ago
Have to agree with the other response. Demanding perfection is not realistic in many situations. If you can get 95% of the way there with 50% of the code/effort, you should do it. Sometimes being productive is knowing when that last percentage just isn't worth it.
mvdan 4y ago
For the first two caveats, I actually agree that we could and should handle ambiguous input. It just hasn't been a priority because doing that properly would be quite a bit of work, and such ambiguous syntax isn't particularly common. See https://github.com/mvdan/sh/issues/686 https://github.com/mvdan/sh/issues/686 for my current thoughts on how to tackle it. The third caveat concerns parsing `export` and `let` as keywords rather than as builtins. Like the README says, this is to properly build the syntax tree without leaving opaque strings as expressions, but also to support `declare foo=(bar)` which wouldn't work if `declare` was treated like any other builtin simple command. How else would you have a static parser handle these two builtins? They are in a bit of an awkward middle ground between builtin and keyword. My instinct is that giving them special treatment in the parser to allow tokens like `(`, while at the same time representing them in the syntax tree with opaque strings as expressions, would be pretty underwhelming to any users of the parser. That said, we already have that problem with `let "foo=123"` for example, where our parser currently represents the expression as the quoted string without going any deeper. https://github.com/mvdan/sh/issues/754#issuecomment-963295749 https://github.com/mvdan/sh/issues/754#issuecomment-96329574... considers doing a second parse stage in the shell interpreter to fix cases like these, though always doing a second parse could get expensive. We _could_ leave all arithmetic expressions as input strings in the parser, and do all the actual parsing when they are evaluated. That would be more compatible with Bash and more consistent. But it would also be less useful to any parser users who don't run into any of these weird edge cases, which aren't common at all, I think. In short, I have some ideas, but I'm not sure at all what's best :) Doing a good job for 99% of users feels better than aiming for 100% compatibility with bash syntax, particularly where bash syntax is a bit weird.

jesusofnazarath 4y ago

[dead]

oweiler 4y ago

I've used it on a small project and it worked beautifully. It even understands and respects Bats syntax.

eschneider 4y ago

Oooh, this looks fun.

jeremy_wiebe 4y ago

And if you use fish, it comes with a built-in formatting function. https://fishshell.com/docs/current/cmds/fish_indent.html https://fishshell.com/docs/current/cmds/fish_indent.html

jmholla 4y ago

Anybody have any luck building the image? It fails for me on podman and docker.

js2 4y ago

I use shfmt and shellcheck together with pre-commit. I like to use the shfmt-py and shellcheck-py pre-commit hooks as opposed to https://github.com/jumanjihouse/pre-commit-hooks https://github.com/jumanjihouse/pre-commit-hooks as they'll install the shfmt/shellcheck prebuilt binaries as needed: https://github.com/maxwinterstein/shfmt-py https://github.com/maxwinterstein/shfmt-py https://github.com/shellcheck-py/shellcheck-py https://github.com/shellcheck-py/shellcheck-py

one-punch 4y ago
An alternative to have both `shfmt` and `shellcheck` and all pre-commit hooks managed (that is, automatically installed until no longer used and garbage collected) is to use https://devenv.sh/ https://devenv.sh/. After `devenv init`, update `devenv.nix` as follows: { pkgs, ... }: { pre-commit.hooks = { shellcheck.enable = true; shfmt.enable = true; }; }

moondev 4y ago

Another gem from the same repo - gosh - pure golang shell This means anywhere golang is installed, including aarch64 Darwin and Windows you can: go run mvdan.cc/sh/v3/cmd/gosh@latest Or things like go run mvdan.cc/sh/v3/cmd/gosh@latest -c 'echo "cross platform shell"; go run github.com/mikefarah/yq/v3@latest r metadata.name <(kubectl get pod my-pod -o yaml)' Pretty awesome stuff, I'm always discovering new ways to use it.

akshayshah 4y ago
At a previous job, my team slowly built an absolutely unmanageable bash script (hundreds of lines, including _many_ complex, multi-line jq incantations). The first step in migrating that script to go was to embed it in a Go binary and run it with gosh. Kudos to Daniel for building such a wonderful package.