search

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
7.27k stars 345 forks source link

syntax: simplify unset-or-null with null default to just unset #849

Closed scop closed 2 years ago

scop commented 2 years ago

If the default for an unset-or-null parameter expansion is null, the or-null part of the expansion is redundant and can be removed. The result is null on match either way.

scop commented 2 years ago

Docker build failure/timeout appears unrelated.

mvdan commented 2 years ago

I'm slightly uneasy about simplifications like these. Are you absolutely sure that this can't break user programs? I wonder if turning an unset to null can matter, for example.

scop commented 2 years ago

I cannot think of a case that this could break.

There certainly is a difference between unset and null valued variables (for example [[ -v varname ]] in bash), but I can't see how this simplification could be used to turn a null to an unset or the other way around -- as said, the expanded value (which is what this simplification is about, it's not changing the variable's value or definedness) is null either way for both unset and null valued variable expansions.

mvdan commented 2 years ago

Alright, SGTM. We can always revert if our thinking here is wrong.