cmd/shfmt: skip zsh scripts when walking directories

[ferrarimarco]

Hi, thanks for this tool! :)

I understand from #120 that zsh is not going to be supported, but the tool (tried with 3.1.0) still wants to format them. I use shfmt in my CI builds to check if the scripts are correctly formatted, and I suppose that if zsh is not supported, it shouldn't try to format it, right?

Example:

#!/usr/bin/env zsh

zstyle ':completion:*' list-colors "${(@s.:.)LS_COLORS}"

Output of shfmt -d .:

script.sh:3:39: parameter expansion requires a literal

This makes my build fail, unless I exclude zsh scripts and call shfmt -d on each file instead. So there's a workaround, but since shfmt does not support zsh, it shouldn't attempt to format them.

What do you think? Thanks!

[mvdan]

Thanks for the issue - I agree this is a bit weird. Right now, if a file has the right extension (in this case, .sh for POSIX Shell), then we don't bother to check the shebang line at all. I think in this case it would be reasonable to do so - we're going to be reading the file anyway, so it's no extra effort, and it can avoid some false positives like the one you found.

[mvdan]

@texastoland see https://github.com/mvdan/sh/issues/120. It would be a lot of work to add support, and a lot of work afterwards to maintain (which would likely fall on just me). So the decision thus far is not to support it.

If someone wants to provide an implementation, and possibly maintain it in the long run, I'm all ears. But noone has stepped up so far :)

[mvdan]

I've hit a bit of a roadblock here, and that's because of the -f flag to find shell scripts under a directory.

If we make shfmt -w . skip *.sh with #!/bin/zsh, that makes sense for formatting, but it would make -f inconsistent, as -f would still list those files.

If we make -f skip those files too, then arguably -f becomes less useful. The point of -f is to find all shell scripts, not just those that shfmt happens to support parsing.

The best alternative here is probably to support all common POSIX shell variants which use .sh extensions in the parser (including zsh), then we can keep both features aligned. Any language that's close enough to POSIX shell should be reasonable to add to the syntax package, and any language that's very different will not use the .sh extension, such as .fish.

[Mellbourn]

I think you should keep including zsh files. In my opinion, the current, somewhat broken support for zsh, is better than no support at all. shfmt still finds many meaningful issues in zsh code.

[mvdan]

It's increasingly likely that we will support zsh in the future, given it being a default for Mac, so I think that's the best long-term solution.

[thomasmerz]

@mvdan A long time has gone by sinvce your last comment… Is zsh already supported?

[mvdan]

@thomasmerz subscribe to https://github.com/mvdan/sh/issues/120 if you want to see the updates when there are any.

[mcandre]

It's increasingly likely that we will support zsh in the future, given it being a default for Mac, so I think that's the best long-term solution.

While we eagerly await zsh support in shfmt, it would fix more projects near term, to temporarily skip file paths of zsh files (*.zsh, *.zshrc, *.zshenv, #!/bin/zsh) in the short term.

As a workaround, I am using a custom sh family finding tool and telling it to skip zsh-related files, before passing them to shfmt:

stank -exInterp zsh . |
    grep -v node_modules |
    xargs shfmt [any other shfmt options here]

This is fragile (breaks on any file paths involving spaces). It requires grep, which may not necessarily be installed in all environments. It requires a separate process, which is wasteful. And it means having to manage a fairly long command in a build configuration file (Makefile, Rakefile, magefile.go, Shakefile.hs, vast.sh, CMakeLists.txt, rez.c/rez.cpp, etc.)

And if any files get accidentally copied to the local tree via go mod vendor, bundle install, or yet other package mangers, then the list of excluded directory patterns grows even longer.

Ideally, shfmt would automatically exclude more files which is cannot handle, and learn to integrate into industry standard tooling like .gitignore, regardless of other future plans for shfmt.