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

Consider an option for spaces before comments #1012

Closed heiner closed 1 year ago

heiner commented 1 year ago

Hey -- amazing project, thank you!

I have been trained at Google to add two spaces between code and a comment on the same line. To take an example from the Google Shell Style Guide:

flags+=' --greeting="Hello world"'  # This won’t work as intended.

This may be an acquired taste but somehow I acquired it.

Right now, shfmt will change this to a single space. Would you consider adding an option for this?

mvdan commented 1 year ago

Interesting that the google shell style guide has that example, but I don't seem to find this rule spelled out anywhere. Is there a reason for that?

Historically I've tried to balance "don't add too many options" with "support standard or common styles", one of them being Google's. That's a tricky balance, as you can imagine. For example, in the manpage we say:

The following formatting flags closely resemble Google's shell style defined in
<https://google.github.io/styleguide/shellguide.html>:

    shfmt -i 2 -ci -bn

which appears to be close enough in many ways.

heiner commented 1 year ago

I agree my case re: Google's Shell style guide is ambiguous as they don't spell out that guideline.

They have that rule explicit in their Python style guide as well as their C++ style guide. Their TypeScript style guide doesn't spell it out explicitly but has //-style comments throughout. The Java and JavaScript style guides don't have two spaces consistently but have them sometimes.

I also agree it's a mistake to add too many options for very specific settings. In this case, perhaps an option one could add would be to leave existing spaces in place instead of enforcing a single space?

mvdan commented 1 year ago

In this case, perhaps an option one could add would be to leave existing spaces in place instead of enforcing a single space?

We have this today in the form of the "keep padding" option, meaning "try to obey the user's choice of spacing between tokens". However, it doesn't work well at all in general, and it's a maintenance nightmare. Which is why it's slated to be removed: https://github.com/mvdan/sh/issues/658

If Google made this form of spacing an explicit property of their shell style, I think it would be worth considering an option, even if the option was something like -style=google or something like that. Being that right now it's not really clear, and the demand doesn't seem to be there (you're the first one to bring it up in years), I'm inclined towards doing nothing for the time being :)

heiner commented 1 year ago

Yep that makes sense, thank you!

mvdan commented 1 year ago

Thanks! Closing this for now then. Happy to reopen if more information comes to light.