fix!: options in .editorconfig not work with shfmt

[adoyle-h]

1. Must pass --filename option to shfmt.

When shfmt read content from stdin, it doesn't know the filename and its extension. So it won't match the patterns "[.sh]" and "[.bash]" in .editorconfig.

For example, my .editorconfig file in project root.

[*.{bash,sh}]
indent_style = space
indent_size = 2
charset = utf-8

### Below are shfmt options ###
# --language-variant
shell_variant      = bash
binary_next_line   = false
# --case-indent
switch_case_indent = true
space_redirects    = true
keep_padding       = false
# --func-next-line
function_next_line = false

You can compare the results between cat ./your-script.sh | shfmt --filename=your-script.sh -d - and cat ./your-script.sh | shfmt -d -.

2. Do not pass any Parser and Printer options like -i/-p/-bn/-l. It will cause the .editorconfig not to be loaded. See https://github.com/mvdan/sh/blob/23633a432f903599a4ce46c30c4337e413a26ef1/cmd/shfmt/main.go#L186-L196

Breaking Change:

Removed shfmtConfig options. Use the .editorconfig options instead of. The .editorconfig options of shfmt refer to https://github.com/mvdan/sh/blob/master/cmd/shfmt/shfmt.1.scd#examples

[adoyle-h]

I recommend to use the .editorconfig to set the shfmt instead of global config in nvim. It is simple and has many benefits.

Related issue: #1161

[chris-reeves]

This change would require users to create an .editorconfig in order to configure shfmt. I think a better approach would be to use .editorconfig to set the formatting options if present, whilst still supporting configuration via the editor. It already does this for indentation if your editor is set up correctly, but will need special handling for the other options. I did consider this as part of the original implementation, but didn't want to overcomplicate things too early! If there's demand then I'm happy to look into this.

[adoyle-h]

The simplest approach is to create a ~/.editorconfig for global configuration for editor, and .editorconfig under project root for project specifics.
Current options of shfmtConfig are incomplete that lacking of "keep_padding", "shell_variant", "indent_style" and "indent_size". And when shfmt changed its options, bash-language-server has to change and release new version. That is overcomplicate.

And the .editorconfig file can be store in git to share with co-workers. But options in editor configuration like vim codes are unshareable.

[chris-reeves]

The simplest approach is to create a ~/.editorconfig for global configuration for editor, and .editorconfig under project root for project specifics.

That's fine as long as people are happy doing that, but not all users will want to - some will want to configure this via their editor as they do for other options.

Current options of shfmtConfig are incomplete that lacking of "keep_padding", "shell_variant", "indent_style" and "indent_size".

• keep_padding was left out as it was deprecated, but my latest PR adds this in as it has been mentioned more than once.
• shell_variant is still not supported via editor configuration as shfmt's detection is pretty good. Again, if there's demand then it can be added.
• Indentation configuration is not specific to shfmt, so you configure this as usual in your editor (or via .editorconfig) and shfmt will use that. My latest PR clarifies this in the README.

And when shfmt changed its options, bash-language-server has to change and release new version. That is overcomplicate.

Fair point. But they don't change frequently so I don't think that's a good reason not to give users the option to configure this via their editor.

And the .editorconfig file can be store in git to share with co-workers. But options in editor configuration like vim codes are unshareable.

Yes, I understand the benefits of .editorconfig and use this myself for some projects, but not everyone wants to do that. I believe in giving users a choice, so think both ways should be supported. I'll raise another PR soon enough to add support for .editorconfig in addition to editor config. :-p

[Shane-XB-Qian]

> And the .editorconfig file can be store in git to share with co-workers. But options in editor configuration like vim codes are unshareable. Yes, I understand the benefits of `.editorconfig` and use this myself for some projects, but not everyone wants to do that. I believe in giving users a choice, so think both ways should be supported. I'll raise another PR soon enough to add support for `.editorconfig` in addition to editor config. :-p

Try 'modeline' of vim, or simple project local '.foo_vimrc' to share, if you like. // not everyone was a fan of editorconfig such thing.

…

-- shane.xb.qian

[adoyle-h]

@chris-reeves Thanks for the detailed explanation. I agree with giving users more choices.

@Shane-XB-Qian How to set these options of shfmt in vim modeline?
The project specific vimrc seems to be a good idea. But it only works for oneself but not co-workers. Because the project specific vimrc may have conflicts with co-workers' global vimrc.

[Shane-XB-Qian]

those just some cfg about indent etc, event no lsp, vim can use shfmt as bash formater, not some fancy.

…

-- shane.xb.qian

[adoyle-h]

The shfmt options in .editorconfig still not work on version 5.3.4. The key point at:

Do not pass any Parser and Printer options like -i/-p/-bn/-l. It will cause the .editorconfig not to be loaded. See mvdan/sh@23633a4/cmd/shfmt/main.go#L186-L196

If we decide to support shfmt options both in .editorconfig and in editor config, it will get complicated.

It is necessary to look up .editorconfig file recursively, and parse the .editorconfig content in bash-lsp. Then the merged the options from editorconfig and shfmtConfig would be passed to shfmt.

The shfmt options in .editorconfig must have higher weight than it in shfmtConfig. Otherwise, the options in .editorconfig would never work because the shfmtConfig always override them.

If only check the .editorconfig file existed and don't parse and merge the .editorconfig options, it is not enough. User may write a .editorconfig for markdown/lua/js files but not for shellscripts.
The bash-lsp must know the shfmt options in .editorconfig for current file to decide whether use shfmtConfig as default or not.

I think it is a bad design that parsing .editorconfig twice in both bash-lsp and shfmt.
What's your advice? @chris-reeves

[chris-reeves]

You're right that we're going to have to be a wee bit clever about this. As you say, just looking for an .editorconfig in the path isn't going to cut it as it may not apply to shell scripts. Likewise we can't blindly apply .editorconfig if there is shell script config in there, because it might only specify indentation config with no shfmt options. My plan is to read and parse the .editorconfig config for the file, combine it with the existing language server config (with a configurable priority) and then pass those options to shfmt. So .editorconfig will only be parsed once (by the language server, not shfmt).

It sounds quite complicated, but should be fairly straightforward to implement. I have this sketched out in my head and should have something by the weekend, assuming I get some time on this over the next couple of evenings.

[chris-reeves]

@adoyle-h #1171 has been merged. If that meets your needs/expectations then I think you can close this PR.

[adoyle-h]

@chris-reeves It looks good to me. Thanks.