From: Shreyansh Paliwal Date: Sun, 01 Mar 2026 15:42:30 GMT Subject: Re: [GSOC][PATCH 1/2] editor: make editor_program local to editor.c Message-ID: <20260301154905.13993-1-shreyanshpaliwalcmsmn@gmail.com> In-Reply-To: > 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/1d43d1d0-bf6b-4806-834e-89f545fab766@gmail.com/ [2]- https://lore.kernel.org/git/xmqqpl63b2tm.fsf@gitster.g/