On Mon, Aug 11, 2025 at 6:17 PM D. Ben Knoble <ben.knoble+github@xxxxxxxxx> 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@xxxxxxxxx> > --- > 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.
