foxundermoon / vs-shell-format

the shellscript、Dockerfile、properties ...... format extension
MIT License
444 stars 47 forks source link

Editorconfig is being ignored #66

Open wayofthepie opened 4 years ago

wayofthepie commented 4 years ago

Hi, thanks for the extension!

shfmt supports using a .editorconfig to define settings. I have the following a the root of the project:

[*]
indent_style = space
indent_size = 4
binary_next_line = true
switch_case_indent = true

This works as expected using shfmt from the terminal, however using this extension those settings are ignored. Just wondering if this is expected?

ferrarimarco commented 4 years ago

Hi! This is really important for projects that are using .editorconfig, otherwise if you've Format on save enabled, you have to disable this plugin entirely because they might conflict with each other, or manually configure shfmt flags from settings.

texastoland commented 4 years ago

I think it's a bug not a feature request. The behavior is different from calling shfmt directly. Could it be because the working directory isn't being passed? Here's an example from the ShellCheck extension:

https://github.com/timonwong/vscode-shellcheck/blob/77b03afd1d8007b48eaa6351e3f9cc3b0c2be544/src/linter.ts#L280-L289

ralish commented 4 years ago

I've submitted a PR which adds this functionality. See PR #102 for more details.

texastoland commented 4 years ago

@ralish I think there was a misunderstanding. You should let shfmt use editorconfig. Instead you're duplicating the functionality just like your format on save feature. The only thing missing was passing the working directory when spawning the command.

ralish commented 4 years ago

@texastoland Spawning shfmt with the current working directory set to that of the file being saved was the approach I originally took, but it's unfortunately not sufficient. As noted in the updated PR, the content of the file being saved is passed to shfmt over stdin (i.e. shfmt only sees the file content, it is not aware of the path of the file nor its name).

Spawning shfmt with the current working directory of the file will thus only apply EditorConfig settings which are applied globally. Options which are specific to certain paths are not applied as shfmt doesn't know the file path required to determine if the options apply. The approach I've taken is admittedly not ideal, but it's working around inherent limitations.

There's work being done in shfmt itself to make dealing with this issue easier as well by providing the file path as a parameter when formatting shell scripts over stdin. When that support lands, the approach I've taken can likely be simplified, provided a shfmt version is being used which has the required support.

texastoland commented 4 years ago

Sorry for not reading! Would a possible workaround be to create a temporary file in the same directory as a buffer? Keep up the good work 👏🏼

ralish commented 4 years ago

@texastoland I don't think I understand the approach you're suggesting?

texastoland commented 4 years ago

I mean is reading from stdin a firm requirement? Could you just update the file in-place with shfmt -w? If you really need a file buffer, you could replace stdin with a generated unique filename in the same directory, copy its formatted contents, then delete the copy. I'm worried trying to duplicate the behavior of shfmt will resolve the immediate problem but yield inconspicuous inconsistencies with the command line.

ralish commented 4 years ago

The thing is we're already inconsistent with the command line. The changes in this PR bring the behaviour of the extension in-line with the results from running shfmt from the command line by ensuring we respect the settings in any present .editorconfig.

Regarding usage of stdin, it is a firm requirement as far as I'm aware. The communication mechanism between VS Code and the extension is via the DocumentFormattingEditProvider interface. Usage of stdin is absolutely the correct approach in my view, as it provides many benefits which are much harder or impossible to implement if operating directly on the underlying file (assuming there is one). Conversely, operating directly on a file means an external utility is making modifications to the user's files concurrent with the editor, sidestepping potentially all manner of user and editor expectations.

mvdan commented 6 months ago

Note that you can use shfmt --filename=path/to/original/file.sh and pass the script contents over stdin. EditorConfig will apply as if you were formatting the file in its original location, and you won't need to re-implement shfmt's own EditorConfig loading logic.