Thank you all. I've made my first round of updates to the documentation, will be taking another pass and looking at adding to functionality. I'd like some help with Walters comments as described below.
On Fri, 28 Oct 2011 12:18:47 -0700, Walter Bright wrote: > On 10/28/2011 6:18 AM, dsimcha wrote: >> Formal review of Jesse Phillips's CSV parser module begins today and >> runs through November . Following that, a vote will take place for one >> week. Please post any comments about this library in this thread. >> >> Docs: >> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html >> >> Code: >> https://github.com/he-the-great/phobos/tree/csv > > First off, mucho thanks to Jesse for writing this. It's an important > module for Phobos. Thank you. Fun fact, many popular languages don't include a CSV parser, C# and Java being notable ones (or at least I haven't found it). > This is a review of the documentation: > > Include this link: http://en.wikipedia.org/wiki/Comma-separated_values > so the programmer can learn more about CSV. Done, added See Also section. > "This parser will loosely follow the RFC-4180" > > How exactly will it differ from RFC-4180? I do not know how to address this. After making this statement I list the criteria. I don't see the need to list where it differs: * Records terminate with LF, CR and not just CRLF. * Using Double Quotes or Comma can be customized. I feel that I summarized well enough that listing yet another would be redundant. > "No exceptions are thrown due to incorrect CSV." > > What happens, then, when the CSV is incorrect? Added list. > "csvText" > > Be more specific about what csvText() returns. You are right. csvText() return a RecordList, but I explain it as though it were the Range. I'll think about what detail here, any suggestions are welcome. > I'm confused about the heading parameter to csvText(). Well I hope some reorganizing and an added example will help. > "the Content of the input" > > What is that? There's no parameter named "Content". I've replace all instances of Content with Contents. It is a template parameter so hopefully this resolves the confusion. > "RecordList" > > "Range which provides access to CSV Records and Fields." > > What is the Range? Do you mean RecordList is a Range? And what does > "provides access" mean? If RecordList is a Range, why is it labeled a > "List"? Considering it is the first documentation line of RecordList, I don't see where the confusion on "what the range is." I've re-written the line, it's shorter. Ok, be prepared for confusion. RecordList is a range of records, it is not named RecordRange because Record is also a range, a range of fields. If you have any suggestions for a better name, such as Document or TabularData Please let me know what you'd prefer. > "Array of the heading contained in the file." > > Now I'm really confused as to what a heading is. I thought it was the > first line? Now there's an array of them? The line did not say "headings," but I have change it to: "Heading from the input in array form." > No examples given for constructors. I provide an example of using RecordList. Any example for the constructors would be the same or similar to the 7 other examples. Is an example needed for both constructors, and maybe an example for front too? > No documentation for front() or empty(). These are range primitives, those programming in D already know "returns the current element of the range." I find it more appropriate to describe what is returned by front() in the description of the range (many ranges have become private and have no documentation at all, not even their existence). I will of course add documentation, but recommendations on non-pointless documentation would be welcome. Or how to handle the different types which can be assigned to Contents. > "Record" > > "Returned by a RecordList when Contents is a non-struct." > > RecordList is a struct, it does not return anything. It is a range, are we not allowed to talk of them as though they were high-level concepts? Do I leave off where you'd obtain a Record (it is private). Should I say "Returned by an instance of RecordList when calling front?" > No examples. It is private, removing doc comment for constructor. > "csvNextToken" > > Example? Added. Hopefully I addressed most of the documentation. Keep them coming.
