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

fileutil: Omit zsh extensions and shebangs #1101

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dcarley
Copy link

@dcarley dcarley commented Oct 9, 2024

testdata: Fix missing zsh files

These lines were being rendered into the contents of shebang-nospace
rather than separate files because they were missing the trailing --
delimiters.

It seems that there's no way to guard against this based txtar docs:

There are no possible syntax errors in a txtar archive.

This change will cause the tests to fail and will be fixed in a
subsequent commit:

> cmpenv stdout find.golden
 diff stdout find.golden
 --- stdout
 +++ find.golden
 @@ -16,6 +16,4 @@
  modify/shebang-space
  modify/shebang-tabs
  modify/shebang-usr-sh
 -skip/ext.zsh
 -skip/shebang-zsh
  symlink/subdir/ext-shebang.sh

 FAIL: testdata/script/walk.txtar:10: stdout and find.golden differ

fileutil: Omit zsh extensions and shebangs

To prevent shfmt from discovering zsh files which aren't currently
supported for formatting.

This fixes #535 and the following test:

> cmpenv stdout find.golden
 diff stdout find.golden
 --- stdout
 +++ find.golden
 @@ -16,6 +16,4 @@
  modify/shebang-space
  modify/shebang-tabs
  modify/shebang-usr-sh
 -skip/ext.zsh
 -skip/shebang-zsh
  symlink/subdir/ext-shebang.sh

 FAIL: testdata/script/walk.txtar:10: stdout and find.golden differ

It could be considered a breaking change to shfmt and fileutil.

There's some discussion in that issue about whether it's beneficial to
match zsh files because they can sometimes be formatted if they
don't contain any zsh-specific syntax, but without any way to explicitly
exclude them from discovery it's more of a hindrance than help until #120 is done.

These lines were being rendered into the contents of `shebang-nospace`
rather than separate files because they were missing the trailing `--`
delimiters.

It seems that there's no way to guard against this based `txtar` docs:

> There are no possible syntax errors in a txtar archive.

This change will cause the tests to fail and will be fixed in a
subsequent commit:

    > cmpenv stdout find.golden
     diff stdout find.golden
     --- stdout
     +++ find.golden
     @@ -16,6 +16,4 @@
      modify/shebang-space
      modify/shebang-tabs
      modify/shebang-usr-sh
     -skip/ext.zsh
     -skip/shebang-zsh
      symlink/subdir/ext-shebang.sh

     FAIL: testdata/script/walk.txtar:10: stdout and find.golden differ
To prevent `shfmt` from discovering `zsh` files which aren't currently
supported for formatting.

This fixes mvdan#535 and the following test:

    > cmpenv stdout find.golden
     diff stdout find.golden
     --- stdout
     +++ find.golden
     @@ -16,6 +16,4 @@
      modify/shebang-space
      modify/shebang-tabs
      modify/shebang-usr-sh
     -skip/ext.zsh
     -skip/shebang-zsh
      symlink/subdir/ext-shebang.sh

     FAIL: testdata/script/walk.txtar:10: stdout and find.golden differ

It could be considered a breaking change to `shfmt` and `fileutil`.

There's some discussion in that issue about whether it's beneficial to
match `zsh` files because they can _sometimes_ be formatted if they
don't contain any zsh-specific syntax, but without any way to explicitly
exclude them from discovery it's more of a hindrance than help.
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

Successfully merging this pull request may close these issues.

cmd/shfmt: skip zsh scripts when walking directories
1 participant