From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sun, 01 Mar 2026 16:22:38 GMT
Subject: Re: [GSOC][PATCH 1/2] editor: make editor_program local to editor.c
Message-ID: <8e657184-ee0b-453a-9f2d-a98080d3582e@gmail.com>
In-Reply-To: <aaQzlE2lsq4WfFxt@fedora>

Hi Burak

On 01/03/2026 13:19, Burak Kaan Karaçay wrote:
> 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.

That's true, but does it really make sense for this config setting 
per-repository? Why would I want to use different editors for different 
repositories in the same process?

Thanks

Phillip

> 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!
> 
> Best,
> Burak Kaan Karaçay
>