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.98k stars 332 forks source link

editorconfig ignored with --ln bats #975

Closed cdleonard closed 1 year ago

cdleonard commented 1 year ago

Here is my editorconfig:

[*]
# like -i=4
indent_style = space
indent_size = 4

main.sh

#! /bin/bash

main() {
    echo aa
}
main "$@"
$ shfmt main.sh
#! /bin/bash

main() {
    echo aa
}
main "$@"
$ shfmt --bats main.sh
#! /bin/bash

main() {
        echo aa
}
main "$@"

As far as I understand editorconfig only works based on file globbing so language dialects should have no impact.

Tested using shfmt 3.4.3-1 from ubuntu 22 and also the mvdan/shfmt docker image

mvdan commented 1 year ago

This is working as documented, per https://github.com/mvdan/sh/blob/d806889c9dcc49e5885942b1de63fae147875dc8/cmd/shfmt/shfmt.1.scd#description:

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.

The "language" flags are parser flags.

The thinking here was that we don't want to support options from multiple sources, because that could easily get confusing. The logic uses either EditorConfig or explicit flags, which I reckon is OK given that we don't have many flags to begin with.

cdleonard commented 1 year ago

This is bizarre - the --bats option is required in order to process .bats files at all without syntax errors. Even if this is "behaving as documented" the fact that editorconfig can't be used for .bats is confusing and never the desired outcome.

Also - the general convention in most tools is that arguments on the command line override config files.

I really don't see why --bats would interact with editorconfig at all - the editorconfig spec says nothing about how to handle language dialects.

mvdan commented 1 year ago

This is bizarre - the --bats option is required in order to process .bats files at all without syntax errors.

That should be working. Can you provide an example?

cdleonard commented 1 year ago

This is bizarre - the --bats option is required in order to process .bats files at all without syntax errors.

That should be working. Can you provide an example?

I checked again and this apparently happens with the shfmt from ubuntu22 (3.4.3-1). I get an syntax error, most likely because of @test.

The dockerized version of shfmt can indeed parse .bats even without --ln bats. So maybe it was fixed recently?

Anyway - I still believe that dialect options should not block editorconfig handling.

mvdan commented 1 year ago

So maybe it was fixed recently?

That sounds right - v3.5.0 swapped the default to -ln=auto to automatically detect languages based on filenames and shebangs. If you can update, that would be the best solution, as you're over a year out of date at this point. Debian has had v3.6.0 for some time, but I'm not sure how Ubuntu handles updates. You could always go install from source or download a binary from GitHub if you prefer to not use Docker.

Anyway - I still believe that dialect options should not block editorconfig handling.

The latest release does the right thing for you here, so I'm inclined to not change documented behavior unless there's a good reason to do so, such as problems encountered in real codebases.