On 09/09/2012 11:54 AM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> [...] >> diff --git a/Documentation/technical/api-string-list.txt >> b/Documentation/technical/api-string-list.txt >> index 9206f8f..291ac4c 100644 >> --- a/Documentation/technical/api-string-list.txt >> +++ b/Documentation/technical/api-string-list.txt >> @@ -68,6 +68,14 @@ Functions >> to be deleted. Preserve the order of the items that are >> retained. >> >> +`string_list_longest_prefix`:: >> + >> + Return the longest string within a string_list that is a >> + prefix (in the sense of prefixcmp()) of the specified string, >> + or NULL if no such prefix exists. This function does not >> + require the string_list to be sorted (it does a linear >> + search). >> + >> `print_string_list`:: > > This may feel like outside the scope of this series, but since this > series will be the main culprit for adding many new functions to > this API in the recent history... > > - We may want to name things a bit more consistently so that people > can tell which ones can be called on any string list, which ones > are sorted list only, and which ones are unsorted one only. > > In addition, the last category _may_ need a bit more thought. > Calling unsorted_string_list_lookup() on an already sorted list > is not a crime---it is just a stupid thing to do.
Yes, this could be clearer. Though I'm skeptical that a naming convention can capture all of the variation without being too cumbersome. Another idea: in string-list.h, one could name parameters "sorted_list" when they must be sorted as a precondition of the function. But before getting too hung up on finery, the effort might be better invested adding documentation for functions that are totally lacking it, like string_list_clear_func() for_each_string_list() for_each_string_list_item() string_list_find_insert_index() string_list_insert_at_index() While we're on the subject, it seems to me that documenting APIs like these in separate files under Documentation/technical rather than in the header files themselves - makes the documentation for a particular function harder to find, - makes it easier for the documentation to get out of sync with the actual collection of functions (e.g., the 5 undocumented functions listed above). - makes it awkward for the documentation to refer to particular function parameters by name. While it is nice to have a high-level prose description of an API, I am often frustrated by the lack of "docstrings" in the header file where a function is declared. The high-level description of an API could be put at the top of the header file. Also, better documentation in header files could enable the automatic generation of API docs (e.g., via doxygen). Is there some reason for the current documentation policy or is it historical and just needs somebody to put in the work to change it? > - Why are these new functions described at the top, not appended at > the bottom? I would have expected either an alphabetical, or a > more generic ones first (i.e. print and clear are a lot "easier" > ones compared to filter and prefix that are very much more > specialized). The order seemed logical to me at the time (given the constraint that functions are grouped by sorted/unsorted/don't-care): print_string_list() is only useful for debugging, so it seemed to belong below the "production" functions. string_list_clear() was already below print_string_list() (which I guessed was because it is logically used last in the life of a string_list) so I left it at the end of its section. My preference would probably have been to move print_string_list() below string_list_clear(), but somebody else made the opposite choice so I decided to respect it. That being said, I don't have anything against a different order. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html