[PATCH 1/2] show: Convert text format to the new self-recursive style
Quoth Dmitry Kurochkin on Jan 31 at 3:26 am: > On Thu, 26 Jan 2012 01:55:25 -0500, Austin Clements > wrote: > > This is all code movement and a smidgen of glue. This moves the > > existing text formatter code into one self-recursive function, but > > doesn't change any of the logic. The next patch will actually take > > advantage of what the new structure has to offer. > > > > Note that this patch retains format_headers_message_part_text because > > it is also used by the raw format. > > --- > > notmuch-show.c | 270 > > +--- > > 1 files changed, 139 insertions(+), 131 deletions(-) > > > > diff --git a/notmuch-show.c b/notmuch-show.c > > index dec799c..6a890b2 100644 > > --- a/notmuch-show.c > > +++ b/notmuch-show.c > > @@ -21,40 +21,17 @@ > > #include "notmuch-client.h" > > > > static void > > -format_message_text (unused (const void *ctx), > > -notmuch_message_t *message, > > -int indent); > > -static void > > -format_headers_text (const void *ctx, > > -notmuch_message_t *message); > > - > > -static void > > format_headers_message_part_text (GMimeMessage *message); > > > > static void > > -format_part_start_text (GMimeObject *part, > > - int *part_count); > > - > > -static void > > -format_part_content_text (GMimeObject *part); > > - > > -static void > > -format_part_end_text (GMimeObject *part); > > +format_part_text (const void *ctx, mime_node_t *node, > > + int indent, const notmuch_show_params_t *params); > > > > static const notmuch_show_format_t format_text = { > > -"", NULL, > > - "\fmessage{ ", format_message_text, > > - "\fheader{\n", format_headers_text, > > format_headers_message_part_text, "\fheader}\n", > > - "\fbody{\n", > > - format_part_start_text, > > - NULL, > > - NULL, > > - format_part_content_text, > > - format_part_end_text, > > - "", > > - "\fbody}\n", > > - "\fmessage}\n", "", > > -"" > > +.message_set_start = "", > > +.part = format_part_text, > > +.message_set_sep = "", > > +.message_set_end = "" > > I guess I missed this during the first review. I think we should > support NULL values for message_set_* members (in a separate patch, I > guess). This would allow us to explicitly initialize only part member > in the above code. I wouldn't want to support this without supporting it for all of the string members of notmuch_show_format_t, which turns out to be a fairly big change. At the end of the show rewrite, all of these other string members will go away, so I'll add support for just these being NULL at that point. > Looks good otherwise. > > Regards, > Dmitry
Re: [PATCH 1/2] show: Convert text format to the new self-recursive style
Quoth Dmitry Kurochkin on Jan 31 at 3:26 am: On Thu, 26 Jan 2012 01:55:25 -0500, Austin Clements amdra...@mit.edu wrote: This is all code movement and a smidgen of glue. This moves the existing text formatter code into one self-recursive function, but doesn't change any of the logic. The next patch will actually take advantage of what the new structure has to offer. Note that this patch retains format_headers_message_part_text because it is also used by the raw format. --- notmuch-show.c | 270 +--- 1 files changed, 139 insertions(+), 131 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index dec799c..6a890b2 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -21,40 +21,17 @@ #include notmuch-client.h static void -format_message_text (unused (const void *ctx), -notmuch_message_t *message, -int indent); -static void -format_headers_text (const void *ctx, -notmuch_message_t *message); - -static void format_headers_message_part_text (GMimeMessage *message); static void -format_part_start_text (GMimeObject *part, - int *part_count); - -static void -format_part_content_text (GMimeObject *part); - -static void -format_part_end_text (GMimeObject *part); +format_part_text (const void *ctx, mime_node_t *node, + int indent, const notmuch_show_params_t *params); static const notmuch_show_format_t format_text = { -, NULL, - \fmessage{ , format_message_text, - \fheader{\n, format_headers_text, format_headers_message_part_text, \fheader}\n, - \fbody{\n, - format_part_start_text, - NULL, - NULL, - format_part_content_text, - format_part_end_text, - , - \fbody}\n, - \fmessage}\n, , - +.message_set_start = , +.part = format_part_text, +.message_set_sep = , +.message_set_end = I guess I missed this during the first review. I think we should support NULL values for message_set_* members (in a separate patch, I guess). This would allow us to explicitly initialize only part member in the above code. I wouldn't want to support this without supporting it for all of the string members of notmuch_show_format_t, which turns out to be a fairly big change. At the end of the show rewrite, all of these other string members will go away, so I'll add support for just these being NULL at that point. Looks good otherwise. Regards, Dmitry ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] show: Convert text format to the new self-recursive style
On Thu, 26 Jan 2012 01:55:25 -0500, Austin Clements wrote: > This is all code movement and a smidgen of glue. This moves the > existing text formatter code into one self-recursive function, but > doesn't change any of the logic. The next patch will actually take > advantage of what the new structure has to offer. > > Note that this patch retains format_headers_message_part_text because > it is also used by the raw format. > --- > notmuch-show.c | 270 > +--- > 1 files changed, 139 insertions(+), 131 deletions(-) > > diff --git a/notmuch-show.c b/notmuch-show.c > index dec799c..6a890b2 100644 > --- a/notmuch-show.c > +++ b/notmuch-show.c > @@ -21,40 +21,17 @@ > #include "notmuch-client.h" > > static void > -format_message_text (unused (const void *ctx), > - notmuch_message_t *message, > - int indent); > -static void > -format_headers_text (const void *ctx, > - notmuch_message_t *message); > - > -static void > format_headers_message_part_text (GMimeMessage *message); > > static void > -format_part_start_text (GMimeObject *part, > - int *part_count); > - > -static void > -format_part_content_text (GMimeObject *part); > - > -static void > -format_part_end_text (GMimeObject *part); > +format_part_text (const void *ctx, mime_node_t *node, > + int indent, const notmuch_show_params_t *params); > > static const notmuch_show_format_t format_text = { > -"", NULL, > - "\fmessage{ ", format_message_text, > - "\fheader{\n", format_headers_text, > format_headers_message_part_text, "\fheader}\n", > - "\fbody{\n", > - format_part_start_text, > - NULL, > - NULL, > - format_part_content_text, > - format_part_end_text, > - "", > - "\fbody}\n", > - "\fmessage}\n", "", > -"" > +.message_set_start = "", > +.part = format_part_text, > +.message_set_sep = "", > +.message_set_end = "" I guess I missed this during the first review. I think we should support NULL values for message_set_* members (in a separate patch, I guess). This would allow us to explicitly initialize only part member in the above code. Looks good otherwise. Regards, Dmitry > }; > > static void > @@ -191,16 +168,6 @@ _get_one_line_summary (const void *ctx, > notmuch_message_t *message) > } > > static void > -format_message_text (unused (const void *ctx), notmuch_message_t *message, > int indent) > -{ > -printf ("id:%s depth:%d match:%d filename:%s\n", > - notmuch_message_get_message_id (message), > - indent, > - notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH), > - notmuch_message_get_filename (message)); > -} > - > -static void > format_message_json (const void *ctx, notmuch_message_t *message, unused > (int indent)) > { > notmuch_tags_t *tags; > @@ -338,26 +305,6 @@ format_message_mbox (const void *ctx, > fclose (file); > } > > - > -static void > -format_headers_text (const void *ctx, notmuch_message_t *message) > -{ > -const char *headers[] = { > - "Subject", "From", "To", "Cc", "Bcc", "Date" > -}; > -const char *name, *value; > -unsigned int i; > - > -printf ("%s\n", _get_one_line_summary (ctx, message)); > - > -for (i = 0; i < ARRAY_SIZE (headers); i++) { > - name = headers[i]; > - value = notmuch_message_get_header (message, name); > - if (value && strlen (value)) > - printf ("%s: %s\n", name, value); > -} > -} > - > static void > format_headers_message_part_text (GMimeMessage *message) > { > @@ -523,78 +470,6 @@ signer_status_to_string (GMimeSignerStatus x) > #endif > > static void > -format_part_start_text (GMimeObject *part, int *part_count) > -{ > -GMimeContentDisposition *disposition = > g_mime_object_get_content_disposition (part); > - > -if (disposition && > - strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0) > -{ > - printf ("\fattachment{ ID: %d", *part_count); > - > -} else { > - > - printf ("\fpart{ ID: %d", *part_count); > -} > -} > - > -static void > -format_part_content_text (GMimeObject *part) > -{ > -const char *cid = g_mime_object_get_content_id (part); > -GMimeContentType *content_type = g_mime_object_get_content_type > (GMIME_OBJECT (part)); > - > -if (GMIME_IS_PART (part)) > -{ > - const char *filename = g_mime_part_get_filename (GMIME_PART (part)); > - if (filename) > - printf (", Filename: %s", filename); > -} > - > -if (cid) > - printf (", Content-id: %s", cid); > - > -printf (", Content-type: %s\n", g_mime_content_type_to_string > (content_type)); > - > -if (g_mime_content_type_is_type (content_type, "text", "*") && > - !g_mime_content_type_is_type (content_type, "text", "html")) > -{ > -
[PATCH 1/2] show: Convert text format to the new self-recursive style
This is all code movement and a smidgen of glue. This moves the existing text formatter code into one self-recursive function, but doesn't change any of the logic. The next patch will actually take advantage of what the new structure has to offer. Note that this patch retains format_headers_message_part_text because it is also used by the raw format. --- notmuch-show.c | 270 +--- 1 files changed, 139 insertions(+), 131 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index dec799c..6a890b2 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -21,40 +21,17 @@ #include "notmuch-client.h" static void -format_message_text (unused (const void *ctx), -notmuch_message_t *message, -int indent); -static void -format_headers_text (const void *ctx, -notmuch_message_t *message); - -static void format_headers_message_part_text (GMimeMessage *message); static void -format_part_start_text (GMimeObject *part, - int *part_count); - -static void -format_part_content_text (GMimeObject *part); - -static void -format_part_end_text (GMimeObject *part); +format_part_text (const void *ctx, mime_node_t *node, + int indent, const notmuch_show_params_t *params); static const notmuch_show_format_t format_text = { -"", NULL, - "\fmessage{ ", format_message_text, - "\fheader{\n", format_headers_text, format_headers_message_part_text, "\fheader}\n", - "\fbody{\n", - format_part_start_text, - NULL, - NULL, - format_part_content_text, - format_part_end_text, - "", - "\fbody}\n", - "\fmessage}\n", "", -"" +.message_set_start = "", +.part = format_part_text, +.message_set_sep = "", +.message_set_end = "" }; static void @@ -191,16 +168,6 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message) } static void -format_message_text (unused (const void *ctx), notmuch_message_t *message, int indent) -{ -printf ("id:%s depth:%d match:%d filename:%s\n", - notmuch_message_get_message_id (message), - indent, - notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH), - notmuch_message_get_filename (message)); -} - -static void format_message_json (const void *ctx, notmuch_message_t *message, unused (int indent)) { notmuch_tags_t *tags; @@ -338,26 +305,6 @@ format_message_mbox (const void *ctx, fclose (file); } - -static void -format_headers_text (const void *ctx, notmuch_message_t *message) -{ -const char *headers[] = { - "Subject", "From", "To", "Cc", "Bcc", "Date" -}; -const char *name, *value; -unsigned int i; - -printf ("%s\n", _get_one_line_summary (ctx, message)); - -for (i = 0; i < ARRAY_SIZE (headers); i++) { - name = headers[i]; - value = notmuch_message_get_header (message, name); - if (value && strlen (value)) - printf ("%s: %s\n", name, value); -} -} - static void format_headers_message_part_text (GMimeMessage *message) { @@ -523,78 +470,6 @@ signer_status_to_string (GMimeSignerStatus x) #endif static void -format_part_start_text (GMimeObject *part, int *part_count) -{ -GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (part); - -if (disposition && - strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0) -{ - printf ("\fattachment{ ID: %d", *part_count); - -} else { - - printf ("\fpart{ ID: %d", *part_count); -} -} - -static void -format_part_content_text (GMimeObject *part) -{ -const char *cid = g_mime_object_get_content_id (part); -GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part)); - -if (GMIME_IS_PART (part)) -{ - const char *filename = g_mime_part_get_filename (GMIME_PART (part)); - if (filename) - printf (", Filename: %s", filename); -} - -if (cid) - printf (", Content-id: %s", cid); - -printf (", Content-type: %s\n", g_mime_content_type_to_string (content_type)); - -if (g_mime_content_type_is_type (content_type, "text", "*") && - !g_mime_content_type_is_type (content_type, "text", "html")) -{ - GMimeStream *stream_stdout = g_mime_stream_file_new (stdout); - g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE); - show_text_part_content (part, stream_stdout); - g_object_unref(stream_stdout); -} -else if (g_mime_content_type_is_type (content_type, "multipart", "*") || -g_mime_content_type_is_type (content_type, "message", "rfc822")) -{ - /* Do nothing for multipart since its content will be printed -* when recursing. */ -} -else -{ - printf ("Non-text part: %s\n", -
[PATCH 1/2] show: Convert text format to the new self-recursive style
This is all code movement and a smidgen of glue. This moves the existing text formatter code into one self-recursive function, but doesn't change any of the logic. The next patch will actually take advantage of what the new structure has to offer. Note that this patch retains format_headers_message_part_text because it is also used by the raw format. --- notmuch-show.c | 270 +--- 1 files changed, 139 insertions(+), 131 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index dec799c..6a890b2 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -21,40 +21,17 @@ #include notmuch-client.h static void -format_message_text (unused (const void *ctx), -notmuch_message_t *message, -int indent); -static void -format_headers_text (const void *ctx, -notmuch_message_t *message); - -static void format_headers_message_part_text (GMimeMessage *message); static void -format_part_start_text (GMimeObject *part, - int *part_count); - -static void -format_part_content_text (GMimeObject *part); - -static void -format_part_end_text (GMimeObject *part); +format_part_text (const void *ctx, mime_node_t *node, + int indent, const notmuch_show_params_t *params); static const notmuch_show_format_t format_text = { -, NULL, - \fmessage{ , format_message_text, - \fheader{\n, format_headers_text, format_headers_message_part_text, \fheader}\n, - \fbody{\n, - format_part_start_text, - NULL, - NULL, - format_part_content_text, - format_part_end_text, - , - \fbody}\n, - \fmessage}\n, , - +.message_set_start = , +.part = format_part_text, +.message_set_sep = , +.message_set_end = }; static void @@ -191,16 +168,6 @@ _get_one_line_summary (const void *ctx, notmuch_message_t *message) } static void -format_message_text (unused (const void *ctx), notmuch_message_t *message, int indent) -{ -printf (id:%s depth:%d match:%d filename:%s\n, - notmuch_message_get_message_id (message), - indent, - notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH), - notmuch_message_get_filename (message)); -} - -static void format_message_json (const void *ctx, notmuch_message_t *message, unused (int indent)) { notmuch_tags_t *tags; @@ -338,26 +305,6 @@ format_message_mbox (const void *ctx, fclose (file); } - -static void -format_headers_text (const void *ctx, notmuch_message_t *message) -{ -const char *headers[] = { - Subject, From, To, Cc, Bcc, Date -}; -const char *name, *value; -unsigned int i; - -printf (%s\n, _get_one_line_summary (ctx, message)); - -for (i = 0; i ARRAY_SIZE (headers); i++) { - name = headers[i]; - value = notmuch_message_get_header (message, name); - if (value strlen (value)) - printf (%s: %s\n, name, value); -} -} - static void format_headers_message_part_text (GMimeMessage *message) { @@ -523,78 +470,6 @@ signer_status_to_string (GMimeSignerStatus x) #endif static void -format_part_start_text (GMimeObject *part, int *part_count) -{ -GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (part); - -if (disposition - strcmp (disposition-disposition, GMIME_DISPOSITION_ATTACHMENT) == 0) -{ - printf (\fattachment{ ID: %d, *part_count); - -} else { - - printf (\fpart{ ID: %d, *part_count); -} -} - -static void -format_part_content_text (GMimeObject *part) -{ -const char *cid = g_mime_object_get_content_id (part); -GMimeContentType *content_type = g_mime_object_get_content_type (GMIME_OBJECT (part)); - -if (GMIME_IS_PART (part)) -{ - const char *filename = g_mime_part_get_filename (GMIME_PART (part)); - if (filename) - printf (, Filename: %s, filename); -} - -if (cid) - printf (, Content-id: %s, cid); - -printf (, Content-type: %s\n, g_mime_content_type_to_string (content_type)); - -if (g_mime_content_type_is_type (content_type, text, *) - !g_mime_content_type_is_type (content_type, text, html)) -{ - GMimeStream *stream_stdout = g_mime_stream_file_new (stdout); - g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE); - show_text_part_content (part, stream_stdout); - g_object_unref(stream_stdout); -} -else if (g_mime_content_type_is_type (content_type, multipart, *) || -g_mime_content_type_is_type (content_type, message, rfc822)) -{ - /* Do nothing for multipart since its content will be printed -* when recursing. */ -} -else -{ - printf (Non-text part: %s\n, - g_mime_content_type_to_string (content_type)); -} -} - -static void