Re: [PATCH 1/2] mailinfo: support rfc3676 (format=flowed) text/plain messages
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Feb 15, 2008, 10:41 UTC
- Message-ID
- <alpine.LSU.1.00.0802151035100.30505@racer.site>
- In-Reply-To
- <1203042077-11385-1-git-send-email-jaysoffian@gmail.com>
Hi,
On Thu, 14 Feb 2008, Jay Soffian wrote:
Show 12 quoted lines
> diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
> index 2600847..deaf92b 100644
> --- a/builtin-mailinfo.c
> +++ b/builtin-mailinfo.c
> @@ -20,6 +20,13 @@ static enum {
> static enum {
> TYPE_TEXT, TYPE_OTHER,
> } message_type;
> +/* RFC 3676 Text/Plain Format and DelSp Parameters */
> +static enum {
> + FORMAT_NONE, FORMAT_FIXED, FORMAT_FLOWED,
> +} tp_format;Why not call it "enum message_format"?
> +static enum {
> + DELSP_NONE, DELSP_YES, DELSP_NO,
> +} tp_delsp;Why not call it "enum message_is_delsp"?
Show 15 quoted lines
> @@ -193,6 +200,18 @@ static int handle_content_type(char *line)
>
> if (strcasestr(line, "text/") == NULL)
> message_type = TYPE_OTHER;
> + else if (strcasestr(line, "text/plain")) {
> + char attr[256];
> + if (slurp_attr(line, "format=", attr) && !strcasecmp(attr, "flowed")) {
> + tp_format = FORMAT_FLOWED;
> + if (slurp_attr(line, "delsp=", attr) && !strcasecmp(attr, "yes"))
> + tp_delsp = DELSP_YES;
> + else
> + tp_delsp = DELSP_NO;
> + }
> + else
> + tp_format = FORMAT_FIXED;Does that mean that the format is only set if the content type is "text/plain"?
Show 14 quoted lines
> @@ -681,6 +700,8 @@ again:
> transfer_encoding = TE_DONTCARE;
> charset[0] = 0;
> message_type = TYPE_TEXT;
> + tp_format = FORMAT_NONE;
> + tp_delsp = DELSP_NONE;
>
> /* slurp in this section's info */
> while (read_one_header_line(line, sizeof(line), fin))
> @@ -770,6 +791,24 @@ static int handle_filter(char *line, unsigned linesize)
> {
> static int filter = 0;
>
> + if (tp_format == FORMAT_FLOWED && !!strcmp(line, "-- \n")) {The !! is unnecessary; please skip it.
Show 6 quoted lines
> + char *cp = line; > + while (*cp == '>' && *cp != 0) > + cp++; > + if (*cp == ' ') > + cp++; > + line = cp;
How about using strchrnul()?
Note: I do not know enough about format=flawed to know why you should skip to "> ". I would have expected "\n", though.
> + if (!!strcmp(line, "-- \n")) {The !! is unnecessary; please skip it.
> + while (*cp != '\n' && *cp !=0) > + cp++;
Again, this is the job for strchrnul().
Show 6 quoted lines
> + if (cp > line && *cp == '\n' && *(cp-1) == ' ') {
> + if (tp_delsp == DELSP_YES)
> + *(cp-1) = '\0';
> + else
> + *cp = '\0';
> + }Or maybe cp[0 - (tp_delsp == DELSP_YES)] = '\0';
But maybe that is too cute.
But another thing struck me here: why setting *cp = '\0'; only if *(cp-1) == ' ', even if tp_delsp != DELSP_YES?
Show 7 quoted lines
> @@ -818,6 +857,7 @@ static void handle_body(void)
>
> switch (transfer_encoding) {
> case TE_BASE64:
> + case TE_QP:
> {
> char *op = line;Did that just slip in, or was this intended. If the latter, is this related to format=flawed, or is it a bug fix in its own right?
Ciao, Dscho