From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Eric Sunshine <sunshine@sunshineco.com>,
Erik Faye-Lund <kusmabite@gmail.com>
Subject: Re: [PATCH v2 1/6] commit: provide a function to find a header in a buffer
Date: Wed, 27 Aug 2014 12:26:56 -0700 [thread overview]
Message-ID: <xmqqsikhd98f.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140827191414.GA7561@peff.net> (Jeff King's message of "Wed, 27 Aug 2014 15:14:14 -0400")
Jeff King <peff@peff.net> writes:
> 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"
> directly.
Fair enough.
> 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.
True.
> 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.
I agree that we do not want to worry too much about the minority
multi-line callers.
I don't mind returning -1 in out_len and have the callers check.
That way will allow callers to easily diagnose this
tree $T
author $GIT_AUTHOR_IDENT
committer $GIT_COMMITTER_IDENT
encoding encoding
unexpected continuation line
log message
as an error; they would just make sure that out_len is not the "this
is continued; you need to parse the rest yourself" value.
Otherwise, the callers need to check value[out_len+1] to see if it
is an SP (value[out_len] must be '\n') to catch the error, I would
think.
next prev parent reply other threads:[~2014-08-27 19:27 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-27 7:55 [PATCH v2 0/6] clean up author parsing Jeff King
2014-08-27 7:56 ` [PATCH v2 1/6] commit: provide a function to find a header in a buffer Jeff King
2014-08-27 17:30 ` Junio C Hamano
2014-08-27 18:00 ` Jeff King
2014-08-27 18:16 ` Jeff King
2014-08-27 19:05 ` Junio C Hamano
2014-08-27 19:14 ` Jeff King
2014-08-27 19:26 ` Junio C Hamano [this message]
2014-08-27 19:38 ` Jeff King
2014-08-27 19:41 ` Junio C Hamano
2014-08-27 7:56 ` [PATCH v2 2/6] record_author_info: fix memory leak on malformed commit Jeff King
2014-08-27 7:56 ` [PATCH v2 3/6] record_author_info: use find_commit_header Jeff King
2014-08-27 7:57 ` [PATCH v2 4/6] use strbufs in date functions Jeff King
2014-08-27 7:57 ` [PATCH v2 5/6] determine_author_info: reuse parsing functions Jeff King
2014-08-27 7:57 ` [PATCH v2 6/6] determine_author_info: copy getenv output Jeff King
2014-08-27 9:06 ` [PATCH v2 0/6] clean up author parsing Christian Couder
2014-08-27 14:18 ` Jeff King
2014-08-27 17:36 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqsikhd98f.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=kusmabite@gmail.com \
--cc=peff@peff.net \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.