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
7.1k stars 336 forks source link

cmd/shfmt: cannot properly integrate with ALE (vim plugin) #889

Closed hbarcelos closed 2 years ago

hbarcelos commented 2 years ago

I'm trying to use shfmt as an ALE fixer. However I'm running into issues with the configs in .editorconfig. Basically shfmt will ignore the configs when run by ALE.

I've opened this issue on ALE and a maintainer made some questions to which I don't know the answer, so I'm seeking help here:

From the ALEInfo I see ALE uses this command:

shfmt -i 2 < /path/to/script.sh

That is different from what you use stand alone:

shfmt -w /path/to/script.sh

Not familiar with shfmt but maybe the .editorconfig won't work if the linter is invoked using stdin as input? or if the -w option is not used?

If we can figure out what command ALE needs to execute to get the functionality you need then it should be easy to modify the fixer to use that command.

mvdan commented 2 years ago

From the man page at https://github.com/mvdan/sh/blob/master/cmd/shfmt/shfmt.1.scd:

If any EditorConfig files are found, they will be used to apply formatting
options. If any parser or printer flags are given to the tool, no EditorConfig
files will be used. A default like *-i=0* can be used for this purpose.

In other words, the tool refuses to mix formatting options from different sources.

I think the easiest solution would be for ALE to avoid giving formatting parameters to shfmt if an editorconfig file exists. If such a config exists, the user can (and probably would) place their shell formatting preferences in that file.

hbarcelos commented 2 years ago

Great! Thank you very much.

One last question: how does shfmt look for .editorconfig files? Does it work when there is such file $PWD even if the script path is outside of it like in the example? ALE seems to use temporary files for that.

mvdan commented 2 years ago

I'm not sure I understand what you mean. EditorConfig files are searched via the chain of parent directories starting at each file being formatted.

hbarcelos commented 2 years ago

ALE does not use the original file path, instead it copies its content to a temporary file in /tmp. I wonder if shfmt takes the current directory in consideration when looking for the .editorconfig or if it only cares about the path of the file.

mvdan commented 2 years ago

The latter - it cares about where the file being formatted is. If you must continue formatting the shell script in a temporary directory, one option would be to also copy EditorConfig files, though that's somewhat clunky.

Is there a particular reason why the formatting must happen on a temporary file copy? For example, if the goal is to not directly overwrite the original file when formatting, you could use shfmt /original/input.sh rather than shfmt -w /original/input.sh, and then the formatted output would be printed to stdout rather than written to disk, and it would find the correct EditorConfig files.

hbarcelos commented 2 years ago

I'm not the author or a maintainer of ALE, I just made some small contributions over the years.

I guess this was an early design decision, however I don't know the reason why it works like that.

I might need to try to bend either shfmt or ALE to work together 😅. Is it possible to pass a reference to an existing .editorconfig file instead of relying on the directory structure?

mvdan commented 2 years ago

You can't really specify one editorconfig file, because the spec explains how multiple can be used at once - they overlap until one "root" editorconfig file is found, or until the root of the filesystem is hit.

I think one possibly reasonable way to bend shfmt would be to replace shfmt /temporary/input.sh with shfmt -filename=/original/input.sh </temporary/input.sh. I haven't tested this yet, but passing an absolute path to -filename should make it then use that path to look for editorconfig files. Note that this flag only works when a shell file is passed as stdin.

hbarcelos commented 2 years ago

I think I managed to make ALE play nice with shfmt using the -filename option.

Thank you very much for your guidance @mvdan.