Re: [PATCH v3 4/4] editor: use standard strvec API to receive environment for external editors

[Eric Sunshine <sunshine@sunshineco.com>]

On Mon, Aug 11, 2025 at 6:17 PM D. Ben Knoble
<ben.knoble+github@gmail.com> wrote:
> Going back to the introduction of the env parameter for the editor in
> 8babab95af (builtin-commit.c: export GIT_INDEX_FILE for launch_editor as
> well., 2007-11-26), we pass a constant array of strings: as the
> surrounding APIs evolved to use strvecs (see 8d7aa4ba6a
> (builtin/commit.c: remove the PATH_MAX limitation via dynamic
> allocation, 2017-01-13) and later 46b225f153 (Merge branch 'jk/strvec',
> 2020-08-10)), the editor code did not.
>
> There is only one caller of all 3 editor APIs that does not pass a NULL
> environment (the same caller for which this parameter was added), and
> it already has a strvec available to use.

I'm confused as to *why* we want to make this change, and the commit
message doesn't seem to explain the motivation or why doing so is
beneficial.

> Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
> ---
> diff --git a/editor.h b/editor.h
> @@ -3,6 +3,7 @@
>  int launch_editor(const char *path, struct strbuf *buffer,
> -                 const char *const *env);
> +                 const struct strvec *env);

Typically, we would want to have the API accept the narrowest type in
order to remain as general as possible, but this change makes it a
requirement that a caller must have the much wider `strvec` type at
hand, which seems counterproductive.