Subject: Re: [GSOC][PATCH 1/2] editor: make editor_program local to editor.c

From: Shreyansh Paliwal <[email-masked]>
Date: Sun, 1 Mar 2026 21:12:30 +0530

> Hi Shreyansh,
>
> I am a GSoC applicant like you. I just wanted to leave my two cents
> here.
>
> On Sun, Mar 01, 2026 at 04:12:58PM +0530, Shreyansh Paliwal wrote:
> >+static char *editor_program;
> >+
> >+int set_editor_program(const char *var, const char *value)
> >+{
> >+	FREE_AND_NULL(editor_program);
> >+	return git_config_string(&editor_program, var, value);
> >+}
> >+
>
> While moving the global variable from 'environment.c' to 'editor.c'
> doesn't cause any behavior change, it still relies on global state.
>
> I think passing a 'struct repository' and using the 'repo_config_get*'
> helpers here might be a more robust approach. I know this means we would
> catch config errors later (right before the editor start up). However,
> since it doesn't seem like it would cause a data loss or serious issues,
> this behavioral change feels like a reasonable trade-off.
>
> Thanks again for the patches!

Hi Burak,

Thanks for the feedback on this, I appreciate you taking the time to look.

I did consider the approach you suggested. Currently, editor_program is
only used within editor.c, and it is I believe a process-wide setting rather
than something tied to a specific repository. Because of that, it did not
seem necessary to add it to struct repository or repo_settings at this stage.

More importantly, my intention for this was to keep original behavior as-is.
As noted in earlier discussions [1][2], maintaining early config validation
is important so that invalid core.editor values are caught early. Moving to
a repo-based lazy lookup would change that.

That said, I agree that there may be a better way to do this refactor,
so I'd be glad to hear more thoughts on this :)

Best,
Shreyansh

[1]- https://lore.kernel.org/git/[email-masked]/
[2]- https://lore.kernel.org/git/[email-masked]/