Re: [GSOC][PATCH 0/2] Remove global state from editor.c

[Tian Yuchen <a3205153416@gmail.com>]

Hi Shreyansh and Burak,

Thanks for the patch.

Reading through discussion, I think both of you highlighted very valid 
constraints:

> While moving the global variable from 'environment.c' to 'editor.c'
> doesn't cause any behavior change, it still relies on global state.

Yes, changing an extern to a static variable doesn't truly remove the 
global state, right?

> 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.

This one also makes sense to me.

However,

> I believe a process-wide setting rather
> than something tied to a specific repository.

I have reservations about this, and I believe this is the most critical 
issue. For instance, we can run:

	git config --local core.editor "nvim"

where the configuration is written in the .git/config of the current 
repository. If core.editor is process-wide, git should not permit the 
existence of a "local" core.editor at all. Since it can be set for 
individual repositories, it should be tied to the specific struct 
repository, right?

A more intuitive case is:

	Repo A: core.editor = vim
	Repo B: core.editor = nvim

For users managing multiple repositories (submodules), it's perfectly 
reasonable to use different editors in different contexts. At least for 
me, I use different configurations for Vim and NVim, and I switch 
between different editors when writing with different languages. (like 
set textwidth=72 for Git? _(:3 ⌒ﾞ)_)

I recently faced the same dilemma migrating git_commit_encoding and 
git_log_output_encoding. I personally believe that adding editor_program 
to repo-settings.c is the best approach.

By doing this:

- We truly eliminate the global state. Each struct repository gets its 
own editor setting.

- We maintain early validation. The config can still be parsed early 
(e.g., during prepare_repo_settings()?)

Thanks again for the patch.

Regards,

Yuchen