Should always use e`${XXXX:-}`, not `${XXXX-}`

[coolaj86]

I noticed in a recent update (where "recent" means "recent to me" - the update could just as well be 2 years ago) that ${XXXX:-} is auto-corrected to ${XXXX-}.

Although the current change does optimize for slighty faster shell runtimes and 1-byte per instance smaller script size, this is a mistake.

It should optimize for readability and change, not file size or microticks.

If I ever go to refactor ${XXXX:-} to be ${XXXX:-default}, this will work wonderfully.

If I go to refactor ${XXXX-} to ${XXXX-default} and I'm not a shell expert, little do I know that I've just introduced a subtle bug into my program. And even if I am a shell expert, if I don't have my maximum awareness, and my screen isn't squeaky-clean from any dust specs (which is a real and practical problem I've come across on more than one occasion), I'll still introduce a bug into my script.

[mvdan]

You make a good case for reverting this change. And you're not the first either; see https://github.com/mvdan/sh/issues/871.

One thing I'll say is that this rewrite doesn't happen by default. You have to opt into it via shfmt --simplify.

You're right that saving one byte is not particularly important. The kinds of transformations that --simplify does are meant to be obviously correct and help make the code a bit easier to understand. They don't happen by default, because they aren't formatting changes, nor should they make scripts potentially harder to maintain, like the example you mention above about future refactors. I had similar worries at first, but couldn't come up with a good example.

Since I want shfmt to lean towards the conservative side in terms of altering scripts, my thinking is to revert that feature.

cc @nemchik @scop for thoughts.

[nemchik]

I would imagine that it could confuse users (like myself) who have seen countless stackoverflow articles and other documentation using the ${XXXX:-} syntax, and very few using the ${XXXX-} syntax. From the perspective of mass find/replace in the future, it would make it easier (simpler) to mass search :-} to find anywhere that has an empty default like ${XXXX:-}. Compared to \w\-\} with regex being required to find ${XXXX-}.

It hasn't presented any major challenges for me personally either way, but I don't think saving 1 character is a necessity (and in my opinion it introduces a less-than-common syntax).

[scop]

To me, this change has always been about readability. I stick with my opinion that ${foo-} is more readable than ${foo:-}, and disagree that the simplification is objectively a mistake.

With regards to refactoring safety, I think this change is as "unsafe" as some others --simplify does. Some of them, such as this one, are done only if some condition holds. When refactoring after the fact so that it no longer does (such as the example given in the initial comment here), the simplification likely needs to be undone, or the thing rewritten some other way for the code to still work. So they simply aren't refactoring safe in that sense.

I do agree that :- is more commonly found in expansions than plain -. When the replacement is a non-empty string, the former is also more commonly "the right thing to do". And there is some value in being consistent with this within a script that has other :-foo uses that need to stay that way. In this particular case it doesn't make a difference wrt correctness, and I don't personally find the other arguments presented here particularly convincing or the related cases supportable all the way. So to me this basically boils down to opinions. But for sure, nothing beats not modifying in terms of being conservative. Then again, as noted, all simplifications are opt in, for a reason I presume :). Anyway, not going to lose any sleep if this gets reverted.

[mvdan]

There are good arguments both in favor and against reverting. And it's true that the "simplify" rewrites are opt-in. I still lean towards reverting on the basis that I've been using shell/bash for years, and even wrote this parser, and I still regularly forget the distinction between :- and - in parameter expansions. So I don't think we should attempt to be clever and use shorter forms, even as an opt-in feature.