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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 10, 2025, 15:10 UTC
Message-ID: <xmqq7by6tkan.fsf@gitster.g>
In-Reply-To: <641aaa9b-2b23-4faf-a13e-f6205e9ef5a2@howdoi.land>

Colin Stagner <ask+git@howdoi.land> writes:

Show 21 quoted lines

> On 9/9/25 21:02, Junio C Hamano wrote:
>> 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 ;-)