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

> On Aug 18, 2026, at 4:42 AM, Karthik Nayak <[email protected]> wrote:
> 
> Kenneth Lorber <[email protected]> writes:
> 
>> Teach git config --edit to show editor failure to the
>> parent process.
>> 
>> Add 2 tests to t1300 to check editor exiting successfully
>> or failing.
>> 
>> Signed-off-by: Kenneth Lorber <[email protected]>
>> ---
>> builtin/config.c  |  5 +++--
>> t/t1300-config.sh | 18 ++++++++++++++++++
>> 2 files changed, 21 insertions(+), 2 deletions(-)
>> 
>> diff --git a/builtin/config.c b/builtin/config.c
>> index 0882899c3f..a166b2131e 100644
>> --- a/builtin/config.c
>> +++ b/builtin/config.c
>> @@ -1291,6 +1291,7 @@ static int cmd_config_remove_section(int argc, const char **argv, const char *pr
>> static int show_editor(struct config_location_options *opts)
>> {
>> 	char *config_file;
>> +	int ret;
>> 
>> 	if (!opts->source.file && !startup_info->have_repository)
>> 		die(_("not in a git directory"));
>> @@ -1313,10 +1314,10 @@ static int show_editor(struct config_location_options *opts)
>> 		else if (errno != EEXIST)
>> 			die_errno(_("cannot create configuration file %s"), config_file);
>> 	}
>> -	launch_editor(config_file, NULL, NULL);
>> +	ret = launch_editor(config_file, NULL, NULL);
>> 	free(config_file);
>> 
>> -	return 0;
>> +	return ret;
>> }
>> 
>> static int cmd_config_edit(int argc, const char **argv, const char *prefix,
>> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
>> index e3f8064889..9a8f852a86 100755
>> --- a/t/t1300-config.sh
>> +++ b/t/t1300-config.sh
>> @@ -1823,6 +1823,24 @@ test_expect_success 'command line overrides environment config' '
>> 	test_cmp expect actual
>> '
>> 
>> +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?

Thank you, this is exactly the cleanup I was looking for.

> 
>> +
>> +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..
> 
>> +
>> test_expect_success 'git config --edit works' '
>> 	git config -f tmp test.value no &&
>> 	echo test.value=yes >expect &&
>> --
>> 2.43.0
> 
> The patch looks good to me otherwise :)

Thank you.