Re: [RFC PATCH 1/1] config: surface editor failure in exit code

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 19, 2026, 20:11 UTC
Message-ID: <xmqqy0e1uazm.fsf@gitster.g>
In-Reply-To: <CAOLa=ZQLgxhq2TVS1AYpRoAc_8AkWVtv_VhEm2HovgEX_cFvWg@mail.gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

Show 11 quoted lines

>> +test_expect_success 'git config --edit successful exit' '
>> +	test_when_finished "rm -rf repo" &&
>> +	git init repo &&
>> +	GIT_EDITOR=true &&
>> +	export GIT_EDITOR &&
>> +	git -C repo config -e &&
>> +	unset GIT_EDITOR
>> +'
>
> Nit: couldn't this be simply `test_env GIT_EDITOR=true git -C repo
> config -e` and avoid the set, export and unset?

No, it should just be a single liner:

	GIT_EDITOR=true git -C repo config -e

I would recommend against use of test_env in most cases, because it introduces a subshell without making it obvious.

Show 10 quoted lines

>> +test_expect_success 'git config --edit failure exit' '
>> +	test_when_finished "rm -rf repo" &&
>> +	git init repo &&
>> +	GIT_EDITOR=false &&
>> +	export GIT_EDITOR &&
>> +	test_must_fail git -C repo config -e &&
>> +	unset GIT_EDITOR
>> +'
>
> Same here..

Even when you truly a need subshell, it is better to spell the subshell invocation out explicitly, i.e.,

    ...
    git init repo &&
    (
	GIT_EDITOR=false &&
	export GIT_EDITOR &&
	test_must_fail git -C repo config -e
    )

rather than using test_env.

But in a case like this where you do not even need a subshell to help you shield your actions from later steps, you can just use "env", like everybody else:

	test_must_fail env GIT_EDITOR=false git -C repo config -e

There are many uses of this pattern.

Thanks.