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

From: Karthik Nayak <karthik.188@gmail.com>
Date: Aug 18, 2026, 08:42 UTC
Message-ID: <CAOLa=ZQLgxhq2TVS1AYpRoAc_8AkWVtv_VhEm2HovgEX_cFvWg@mail.gmail.com>
In-Reply-To: <20260817211936.2943278-2-keni@his.com>

Kenneth Lorber <keni@his.com> writes:

Show 53 quoted lines

> 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 <keni@his.com>
> ---
>  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?

Show 9 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..

Show 6 quoted lines

> +
>  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 :)