On Fri, Dec 10, 2010 at 03:22:18PM -0800, Junio C Hamano wrote: > Yann Dirson <ydirson@xxxxxxxxxx> writes: > > > ... I want to get my focus back to > > bulk-rename/builk-rm patches, which will make heavy use of this API. > > Final comment. As the primary thing you want to use this is to change the > way how the rename_dst/rename_src tables are managed, and these are both > tables sorted by a string, I suspect a more reasonable might be to first > updated them to use string-list API and add to that API whatever necessary > features you might need, if any. It sounds reasonable to build on existing stuff (furthermore, the string-list binary search is one I had missed). Using string-lists here however will imply some tradeofs: * the additional char* pointer in every list element is possibly not so high a price to pay * using the "util" pointer for the payload will make memory management even more hairy (eg. "util" as a pointer to a struct which contains a pointer to a diff_filespec). Convenience wrappers will be highly needed, and will also be required to keep calls to lookup/insert readable, when the elements we deal with are not strings but indeed the "util" stuff. All in all, looks that the data-structure needed should have a higher focus on the "util" field than string-list has. Features that seem to miss from string-list today (for the "dir-rename" series) include: * custom string-comparison function (ie. prefix comparison): that would not be so difficult to generalize by adding a cmp_func parameter to get_entry_index(). That would imply changing widely-used API funcs like string_list_lookup() to shallow wrappers around variants that also take a cmp_func argument. * lists indexed by 2 strings (bulkmove_candidates): could be replaced by using string-lists of string-lists instead, but I'm not sure the result would be that great I still have mixed feelings about all of this. -- Yann -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html