Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spaces_around_brackets for bash arrays? #525

Closed
tianon opened this issue Mar 11, 2020 · 3 comments
Closed

spaces_around_brackets for bash arrays? #525

tianon opened this issue Mar 11, 2020 · 3 comments

Comments

@tianon
Copy link

@tianon tianon commented Mar 11, 2020

I searched and couldn't find an existing issue about space around/inside single-line bash arrays or spaces_around_brackets (from EditorConfig) -- shell can't syntactically support outside (and thus both is also out), but inside could be supported to turn something like a=(foo bar baz) into a=( foo bar baz ) (and likely apply to arithmetic too? (( 1 + 2 )) in place of ((1 + 2))).

It might also make sense to officially consider spaces_around_operators?
(although I personally care much less about it, given I'm pro-spaces_around_operators = true 😅)

(ref https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties, although I'm guessing I don't need to link that to you 😄)

Thanks for creating such a neat tool (and for handling so many edge cases of POSIX/Bash). 👍 💪

@mvdan

This comment has been minimized.

Copy link
Owner

@mvdan mvdan commented Mar 11, 2020

I had actually only looked into "widely supported" options, and not the rest. The two options you bring up are described as:

This is simply a brainstorm of domain-specific properties that could be supported by some tools that rely on EditorConfig files.

So they don't seem particularly widespread or endorsed by the project :)

In general, the tool has only gained options for choices that annoyed too many users, or made it impossible to support widespread style guides (like Google's). We already have six formatting options, and I personally think that's enough for the foreseeable future. I understand that we can't be gofmt with one single style because that ship sailed for shell decades ago, but I also want to avoid man indent with its nearly hundred options.

If the reason behind this issue is "I found these semi-standard EditorConfig properties, why don't we support them", the answer is that I don't intend to support all standard properties. For example, charset and end_of_line make no sense for shell scripts; they are all unix-y. Some of our options mapped nicely to standard properties, like -i to indent_style and indent_size, and the ones that didn't were added as custom properties. I don't think we should go beyond that for the time being.

If the reason is instead "I think this should be a formatting option", some background info would be good. Does any shell style guide enforce this kind of spacing? Are there any large projects that use this kind of style?

One thought that comes to mind is -sr. The default format, in general, prefers to avoid spaces around unary and open/close operators. Perhaps in the future we could consider folding -sr into "space all operators".

@tianon

This comment has been minimized.

Copy link
Author

@tianon tianon commented Mar 11, 2020

My own reason was as simple as "this is one of the very few places shfmt didn't match my personal style" so I figured it wouldn't hurt to have something on the record -- I honestly only looked up the .editorconfig equivalent to have a more sane way to talk about what I saw and essentially have a discussion around whether you find it something worth supporting. Fully hear you on having a million options!

I like the idea of -sr becoming "space around operators" but I'm definitely satisfied with closing this, if that's your preference (especially with such a comprehensive justification as you've now added). 👍 ❤️

@mvdan mvdan closed this in 8bfaa27 Mar 14, 2020
@mvdan

This comment has been minimized.

Copy link
Owner

@mvdan mvdan commented Mar 14, 2020

Thanks - I agree we should close this for now. I don't have short-term plans for a v4, but when that time comes, I'd find it reasonable to coalesce all of this into -sr. I'd prefer to not tackle this for v3, given that it doesn't seem urgent, and that I can't break backwards compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.