On 1/7/2021 2:57 PM, Junio C Hamano wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>
>>> It could be possible to replace all references to 'istate' with
>>> 'repo->index' but the patches get slightly more messy. I also
>>> think the code looks messier, but you do make a good point that
>>> there is no concrete reason to separate the two.
>>
>> I agree that it would make the code a bit noisier (to read) if
>> `istate` is eliminated from the callback structure, however, even
>> though I didn't originally feel strongly one way or the other about
>> having both `repo` and `istate` in the structure, I'm now leaning more
>> toward seeing `istate` eliminated. My one (big) concern with `istate`
>> is that it confuses readers into wondering whether `istate` and
>> `repo->istate` will ever be different.
>
> Some applications may want to work on more than one in-core index at
> the same time (perhaps a merge strategy may want to keep a copy of
> the original index and update a second copy with the result of the
> merge), and it may be useful for such applications if 'repo->istate'
> does not have to be the in-core index instance to be worked on. So
> things that go in libgit.a may want to allow such distinction.
>
> But what goes in builtin/ is a different story. As long as this
> application has no need for such a feature and will always work on
> the primary in-core index, prepared for the in-core repository
> structure for convenience, it may not worth it to support such a
> feature that no callers benefit from.
I'll try to restructure my patches to do the following order:
1. replace compatibility macros with static global references, except
i. use 'istate' in the methods that don't need a repository.
ii. use 'repo->index' in the methods that need a repository.
2. replace static globals with method parameters.
i. drop 'istate' static global with method parameters. Methods that
have a repo will pass 'repo->index' to these methods.
ii. drop 'repo' static global with method parameters.
3. replace static globals in callback methods using 'repo->index',
where 'repo' is a member of the callback_data struct.
That should keep the structure as presented in v2 while also avoiding
this question of "can istate differ from repo->index?"
Thanks,
-Stolee
