Re: [PATCH v2 1/6] commit: provide a function to find a header in a buffer

[Jeff King <peff@peff.net>]

On Wed, Aug 27, 2014 at 10:30:22AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > +const char *find_commit_header(const char *msg, const char *key, size_t *out_len)
> [...]
> 
> Hmph.  Does this have to worry about continuation lines in the
> header part e.g. mergetag?  If the original in pretty.c was only
> about the encoding, it may not have mattered, but now because it is
> made public, it may matter more.

If you mean parsing past continuation lines, then no, we do not need to
worry. We go line by line and look for the key at the beginning of a
line, so we would skip past any continuation lines.

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.

That may be something some callers want, but they should build it
separately around this find_commit_header, so that callers that want a
single line (like "encoding" or "author") do not have to pay the price.
I didn't bother building it out here since there are no callers which
want it yet (though I did not look at the mergetag code, which could
possibly be converted).

In the meantime, I hoped this comment would suffice for any callers to
know whether it was suitable:

+/*
+ * Search the commit object contents given by "msg" for the header "key".
+ * Returns a pointer to the start of the header contents, or NULL. The length
+ * of the header, up to the first newline, is returned via out_len.
+ *
+ * Note that some headers (like mergetag) may be multi-line. It is the caller's
+ * responsibility to parse further in this case!
+ */
+extern const char *find_commit_header(const char *msg, const char *key,
+                                     size_t *out_len);

-Peff