Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > Questions: > > 1. Should I be calling read-tree here with run_command_v_opt(), or is > there some internal API I should be using? The internal is unpack_trees(), which is usabe as a library-ish API reasonably cleanly and easily. For a project large enough where the perforce can become an issue, the overhead to spawn read-tree as an external process would be dwarfed by the cost of real processing of merging multiple trees into an in-core index, but it involves two extra writing and reading the index file back and forth compared to the approach to use unpack_trees() to do everything in core. As long as the "now make sure that the contents of the index file is that of the tree of the N-th parent" code is cleanly isolated in the implementation, it is probably better to refrain from premature optimization. > 2. Currently merge-ours is just a no-op since we take the current HEAD, > but maybe it would be cleaner to implement it in terms of this > thing, also to get immediate test coverage for all the -s ours > stuff. We'd be reading the tree redundantly into the index, but > maybe it's worth it for the coverage... I doubt that it would be a sensible approach. If anything, even if we lived in an alternate universe where "-s ours" weren't invented and "-s become-nth-tree -W $N" feature gets first introduced, I would imagine that we would introduce a code to special case "-s ours" (aka "take the current HEAD") as an obvious optimization for the common and trivial case, essentially splitting the "unification" you are suggesting here. > 3. Is there a better name for this than -s theirs? Maybe `-s nth -X N`? I tend to agree that "-s thiers -X N" is horrible; "-s ours -X N" would slightly be a better choice as it does not have to introduce a new option. "-s nth -X N" does not sound all that bad. "Does this feature make sense?" was not among the questions listed, and I am not ready to answer to it yet anyway, so ...
