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