Re: [PATCH v2] contrib/subtree: fix split with squashed subtrees

Colin Stagner [off-list ref] writes:

On 9/9/25 21:02, Junio C Hamano wrote:

quoted

Besides, "if test -n ${subtree_dir-}" without colon would be the
more proper way for those who care about "set -u", wouldn't it?  It
is not that you want to substitute with an empty string that comes
between that "-" and "}" when subtree_dir is unset or set to empty.
You are preparing for the case where the variable is truly not set,
and the variable being set to an empty string is not something you
are worried about.

Yes, "test -n ${subtree_dir-}" is definitely the more correct expression.

At the very real risk of embarrassing myself in public today... in the
particular case of a "test -n," is there actually an appreciable
difference? Either way, the output of the substitution is empty if the
input is empty or undefined. Here, "test -n ${subtree_dir:-}" is
merely less efficient. Right?

The difference between "${x:-}" vs "${x-}" really starts to matter if
you want to permit the empty string (or not). It also matters if you
call a command that has side effects.

(And in the context of this patch, neither are necessary.)

Correct.  There is no practical difference.

Your explanation for using the "default values" parameter expansion
in this script, knowing that "set -u" is not in use, being it is out
of inertia, I would have expected them to be written in a way that
is suitable when "set -u" is in use, which is without colon.  Doing
something "different" on a variable that is set but set to an empty
string is not something you would want to do to deal with "set -u",
so I found it strange to see the colon there.

There is no practical difference, since the "default value"
specified is an empty string, so a variable set to an empty string
will use the empty string between ":-" and "}" instead of its value
that is another empty string, and you can tell these two empty
strings apart in the result ;-)