On Wed, Aug 27, 2014 at 10:30:22AM -0700, Junio C Hamano wrote: > Jeff King <p...@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 -- 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
