Subject: Re: [PATCH v4 09/21] strbuf: give strbuf_getline() to the "most text friendly" variant

From: Junio C Hamano <[email-masked]>
Date: Fri, 15 Jan 2016 10:02:09 -0800

Eric Sunshine <[email-masked]> writes:

>>  /**
>>   * Read a line from a FILE *, overwriting the existing contents
>> - * of the strbuf. The second argument specifies the line
>> - * terminator character, typically `'\n'`.
>> + * of the strbuf.  There are three public functions with this
>> + * function signature, with different line termination convention.
>
> s/public// perhaps?
>
> Also, is it worth worrying that the comment may become outdated due to
> the mentioned "three"? Perhaps s/three/several/? Or:
>
>     The family of strbuf_getline*() functions share the same
>     signature, but have different line termination conventions.

I like this one the best, and that is not because I foresee "three"
to be changing (strbuf_getdelim() could become public again, but it
has a different signature), but because it is concise and to the
point.

>>   * Reading stops after the terminator or at EOF.  The terminator
>>   * is removed from the buffer before returning.  Returns 0 unless
>>   * there was nothing left before EOF, in which case it returns `EOF`.
>>   */
>> -extern int strbuf_getline(struct strbuf *, FILE *, int);
>> -
>>
>>  typedef int (*strbuf_getline_fn)(struct strbuf *, FILE *);
>>
>> +/* Uses LF as the line terminator */
>>  extern int strbuf_getline_lf(struct strbuf *sb, FILE *fp);
>> +
>> +/* Uses NUL as the line terminator */
>>  extern int strbuf_getline_nul(struct strbuf *sb, FILE *fp);
>
> This documentation could have been included in patch 3/21.

Perhaps.  I'll see what the best tweak would be.

Thanks.