mvdan / sh

A shell parser, formatter, and interpreter with bash support; includes shfmt
https://pkg.go.dev/mvdan.cc/sh/v3
BSD 3-Clause "New" or "Revised" License
6.97k stars 332 forks source link

cmd/shfmt: document how --filename relates to EditorConfig support #1055

Closed tcrawford-figure closed 4 months ago

tcrawford-figure commented 5 months ago

I'm imagining something like the following:

shfmt --editorconfig-path=../relative/path/to/.editorconfig foo.sh
shfmt --editorconfig-path=/absolute/path/to/.editorconfig foo.sh

This becomes useful when integrating shfmt into other applications/plugins. Please see the following PR, if interested, for Gradle plugin integration: https://github.com/diffplug/spotless/pull/2031.

A different flag name can be used than what I provided, but this at least gives the gist of what I'm looking for.

mvdan commented 4 months ago

Not sure I understand; the entire point of editorconfig files is that they apply to the files relative to their location. When would you want to use this flag?

tcrawford-figure commented 4 months ago

@mvdan There seems to be some potentially unexpected behavior with how .editorconfig is respected.

Take the following .editorconfig for example:

root = true

[*]
charset = utf-8

[*.sh]
indent_style = space
indent_size = 2
space_redirects = true
switch_case_indent = true

When I have a file named foo.sh with the following contents:

#!/usr/bin/env bash

function foo()
{
  if [ -x $file ]
  then
myArray=(item1 item2 item3)
  elif [ $file1 -nt $file2 ]
  then
    unset myArray
          else
    echo "Usage: $0 file ..."
  fi
}

for (( i = 0; i <   5; i++  ))
do
      read -p r
print -n $r
                    wait $!
done

I can successfully format this file, with .editorconfig respected, by invoking:

shfmt foo.sh

However, if I instead add the contents of foo.sh to stdin, the .editorconfig is not respected:

shfmt < foo.sh

Given this, what I have noticed is if I provide a --filename foo.sh to shfmt, the .editorconfig is respected:

shfmt < foo.sh --filename foo.sh

I'm not sure if this is a known feature/issue/limitation? If it is, I think that an update to the man page that mentions that this flag is required when providing content to stdin in order for the nearest .editorconfig to be respected would be a good change.

So this is where my original request came from for having an --editorconfig-path flag. It may not be necessary and the --filename flag may be the preferred method to fix this.

mvdan commented 4 months ago

That is all working as intended. When you pass in a file via stdin, the filename is not known. We added the filename flag precisely for this purpose (and for related others such as error positions).

tcrawford-figure commented 4 months ago

@mvdan Thanks for confirming! Would documentation in the man page regarding this requirement be worth adding?

mvdan commented 4 months ago

Oops, this was indeed an undocumented fact. Opening a PR.

tcrawford-figure commented 4 months ago

@mvdan perfect! Thank you!