-
-
Notifications
You must be signed in to change notification settings - Fork 353
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
Should always use e${XXXX:-}
, not ${XXXX-}
#970
Comments
You make a good case for reverting this change. And you're not the first either; see #871. One thing I'll say is that this rewrite doesn't happen by default. You have to opt into it via You're right that saving one byte is not particularly important. The kinds of transformations that Since I want shfmt to lean towards the conservative side in terms of altering scripts, my thinking is to revert that feature. |
I would imagine that it could confuse users (like myself) who have seen countless stackoverflow articles and other documentation using the 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). |
To me, this change has always been about readability. I stick with my opinion that With regards to refactoring safety, I think this change is as "unsafe" as some others I do agree that |
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 |
Changing this to ${FOO-} was done by 3.6.0, but has since been reverted. See mvdan/sh#970 Signed-off-by: Jan Dubois <[email protected]>
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.The text was updated successfully, but these errors were encountered: