On Wed, Aug 27, 2014 at 12:05:06PM -0700, Junio C Hamano wrote:

> > If you mean including continuation lines in the output, I don't think
> > that's a good idea here. It would mean the function would have to copy
> > the value out (to get rid of the continuation whitespace) rather than
> > point directly into the msg buffer.
> I meant the counting of out_len.  You do not copy the contents for
> the caller for a single line case either, so I wouldn't expect it.
> You locate where the stuff begins to make it easier for the caller
> to read or copy, and the caller may choose to stop reading from the
> location up to out_len bytes, or it may choose to ignore out_len and
> stop at the first newline.  For the latter kind of caller, it does
> not matter if out_len does not point at the end of the "field"
> (i.e. for a continued-line case), but for the former, wouldn't it be
> more useful if out_len told where the end of the "field" is?

I think they are a direct tradeoff. If you include only the first line,
then callers who want multiple lines have to keep parsing.  If you
include multiple lines, then callers who care only about the first line
will have to re-find the newline rather than just using "out_len"

I suppose you could argue that people who are only expecting one line
(e.g., "encoding") should just assume that out_len ends at the first
line. For correctly-formatted commits, that works the same under either
scheme. For a broken commit where "encoding" _is_ multi-line, one case
would ignore the continued bits and the other case would return an
unexpected encoding value with newlines in it. The choice probably
doesn't matter much in practice.

Mostly I just punted on it with a comment since I did not plan to add
any multi-line callers, and I figured we could sort it out then. If you
feel strongly, it should be pretty easy to check for continuation and
extend out_len if necessary.

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

Reply via email to