* [PATCH 0/2] Rewrite text show format
@ 2012-01-26 6:55 Austin Clements
2012-01-26 6:55 ` [PATCH 1/2] show: Convert text format to the new self-recursive style Austin Clements
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Austin Clements @ 2012-01-26 6:55 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
And now the real fun begins. This series translates the text
formatter into the new format style in two steps: the first patch is a
big diff but just shuffles code and the second actually takes
advantage of the new structure.
This incorporates Dmitry's comments on the RFC patch series from
id:87lioxna2n.fsf@gmail.com .
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] show: Convert text format to the new self-recursive style
2012-01-26 6:55 [PATCH 0/2] Rewrite text show format Austin Clements
@ 2012-01-26 6:55 ` Austin Clements
2012-01-30 23:26 ` Dmitry Kurochkin
2012-01-26 6:55 ` [PATCH 2/2] show: Simplify new text formatter code Austin Clements
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Austin Clements @ 2012-01-26 6:55 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
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
-format_part_end_text (GMimeObject *part)
-{
- GMimeContentDisposition *disposition;
-
- disposition = g_mime_object_get_content_disposition (part);
- if (disposition &&
- strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
- {
- printf ("\fattachment}\n");
- }
- else
- {
- printf ("\fpart}\n");
- }
-}
-
-static void
format_part_start_json (unused (GMimeObject *part), int *part_count)
{
printf ("{\"id\": %d", *part_count);
@@ -844,6 +719,139 @@ format_part_content_raw (GMimeObject *part)
}
static void
+format_part_text (const void *ctx, mime_node_t *node,
+ int indent, const notmuch_show_params_t *params)
+{
+ /* The disposition and content-type metadata are associated with
+ * the envelope for message parts */
+ GMimeObject *meta = node->envelope_part ?
+ GMIME_OBJECT (node->envelope_part) : node->part;
+ GMimeContentType *content_type = g_mime_object_get_content_type (meta);
+ int i;
+
+ if (node->envelope_file) {
+ notmuch_message_t *message = node->envelope_file;
+ const char *headers[] = {
+ "Subject", "From", "To", "Cc", "Bcc", "Date"
+ };
+ const char *name, *value;
+ unsigned int i;
+
+ printf ("\fmessage{ ");
+ 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));
+
+ printf ("\fheader{\n");
+
+ 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);
+ }
+ printf ("\fheader}\n");
+ } else {
+ GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (meta);
+ const char *cid = g_mime_object_get_content_id (meta);
+
+ if (disposition &&
+ strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
+ {
+ printf ("\fattachment{ ID: %d", node->part_num);
+
+ } else {
+
+ printf ("\fpart{ ID: %d", node->part_num);
+ }
+
+ if (GMIME_IS_PART (node->part))
+ {
+ const char *filename = g_mime_part_get_filename (GMIME_PART (node->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 (node->envelope_part) {
+ GMimeMessage *message = GMIME_MESSAGE (node->part);
+ InternetAddressList *recipients;
+ const char *recipients_string;
+
+ printf ("\fheader{\n");
+ printf ("Subject: %s\n", g_mime_message_get_subject (message));
+ printf ("From: %s\n", g_mime_message_get_sender (message));
+ recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
+ recipients_string = internet_address_list_to_string (recipients, 0);
+ if (recipients_string)
+ printf ("To: %s\n", recipients_string);
+ recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
+ recipients_string = internet_address_list_to_string (recipients, 0);
+ if (recipients_string)
+ printf ("Cc: %s\n", recipients_string);
+ printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
+ printf ("\fheader}\n");
+ }
+
+ if (!node->envelope_file) {
+ 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 (node->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));
+ }
+ }
+
+ if (GMIME_IS_MESSAGE (node->part))
+ printf ("\fbody{\n");
+
+ for (i = 0; i < node->nchildren; i++)
+ format_part_text (ctx, mime_node_child (node, i), indent, params);
+
+ if (GMIME_IS_MESSAGE (node->part))
+ printf ("\fbody}\n");
+
+ if (node->envelope_file) {
+ printf ("\fmessage}\n");
+ } else {
+ GMimeContentDisposition *disposition;
+
+ disposition = g_mime_object_get_content_disposition (meta);
+ if (disposition &&
+ strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
+ {
+ printf ("\fattachment}\n");
+ }
+ else
+ {
+ printf ("\fpart}\n");
+ }
+ }
+}
+
+static void
show_message (void *ctx,
const notmuch_show_format_t *format,
notmuch_message_t *message,
--
1.7.7.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] show: Simplify new text formatter code
2012-01-26 6:55 [PATCH 0/2] Rewrite text show format Austin Clements
2012-01-26 6:55 ` [PATCH 1/2] show: Convert text format to the new self-recursive style Austin Clements
@ 2012-01-26 6:55 ` Austin Clements
2012-01-30 14:59 ` Tomi Ollila
2012-01-30 23:37 ` Dmitry Kurochkin
2012-02-04 21:24 ` [PATCH v2 0/2] Rewrite text show format Austin Clements
2012-02-14 14:49 ` [PATCH " Adam Wolfe Gordon
3 siblings, 2 replies; 14+ messages in thread
From: Austin Clements @ 2012-01-26 6:55 UTC (permalink / raw)
To: notmuch; +Cc: tomi.ollila
This makes the text formatter take advantage of the new code
structure. The previously duplicated header logic is now unified,
several things that we used to compute repeatedly across different
callbacks are now computed once, and the code is simpler overall and
32% shorter.
Unifying the header logic causes this to format some dates slightly
differently, so the two affected test cases are updated.
---
notmuch-show.c | 88 +++++++++++++--------------------------------------
test/crypto | 2 +-
test/thread-naming | 16 +++++-----
3 files changed, 32 insertions(+), 74 deletions(-)
diff --git a/notmuch-show.c b/notmuch-show.c
index 6a890b2..30f6501 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -727,67 +727,48 @@ format_part_text (const void *ctx, mime_node_t *node,
GMimeObject *meta = node->envelope_part ?
GMIME_OBJECT (node->envelope_part) : node->part;
GMimeContentType *content_type = g_mime_object_get_content_type (meta);
+ const notmuch_bool_t leaf = GMIME_IS_PART (node->part);
+ const char *part_type;
int i;
if (node->envelope_file) {
notmuch_message_t *message = node->envelope_file;
- const char *headers[] = {
- "Subject", "From", "To", "Cc", "Bcc", "Date"
- };
- const char *name, *value;
- unsigned int i;
- printf ("\fmessage{ ");
- printf ("id:%s depth:%d match:%d filename:%s\n",
+ part_type = "message";
+ printf ("\f%s{ id:%s depth:%d match:%d filename:%s\n",
+ part_type,
notmuch_message_get_message_id (message),
indent,
notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
notmuch_message_get_filename (message));
-
- printf ("\fheader{\n");
-
- 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);
- }
- printf ("\fheader}\n");
} else {
GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (meta);
const char *cid = g_mime_object_get_content_id (meta);
+ const char *filename = leaf ?
+ g_mime_part_get_filename (GMIME_PART (node->part)) : NULL;
if (disposition &&
strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
- {
- printf ("\fattachment{ ID: %d", node->part_num);
-
- } else {
-
- printf ("\fpart{ ID: %d", node->part_num);
- }
-
- if (GMIME_IS_PART (node->part))
- {
- const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
- if (filename)
- printf (", Filename: %s", filename);
- }
+ part_type = "attachment";
+ else
+ part_type = "part";
+ printf ("\f%s{ ID: %d", part_type, node->part_num);
+ 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 (node->envelope_part) {
+ if (GMIME_IS_MESSAGE (node->part)) {
GMimeMessage *message = GMIME_MESSAGE (node->part);
InternetAddressList *recipients;
const char *recipients_string;
printf ("\fheader{\n");
+ if (node->envelope_file)
+ printf ("%s\n", _get_one_line_summary (ctx, node->envelope_file));
printf ("Subject: %s\n", g_mime_message_get_subject (message));
printf ("From: %s\n", g_mime_message_get_sender (message));
recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
@@ -800,9 +781,11 @@ format_part_text (const void *ctx, mime_node_t *node,
printf ("Cc: %s\n", recipients_string);
printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
printf ("\fheader}\n");
+
+ printf ("\fbody{\n");
}
- if (!node->envelope_file) {
+ if (leaf) {
if (g_mime_content_type_is_type (content_type, "text", "*") &&
!g_mime_content_type_is_type (content_type, "text", "html"))
{
@@ -810,45 +793,20 @@ format_part_text (const void *ctx, mime_node_t *node,
g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
show_text_part_content (node->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
- {
+ } else {
printf ("Non-text part: %s\n",
g_mime_content_type_to_string (content_type));
}
}
- if (GMIME_IS_MESSAGE (node->part))
- printf ("\fbody{\n");
-
for (i = 0; i < node->nchildren; i++)
format_part_text (ctx, mime_node_child (node, i), indent, params);
- if (GMIME_IS_MESSAGE (node->part))
+ if (GMIME_IS_MESSAGE (node->part)) {
printf ("\fbody}\n");
-
- if (node->envelope_file) {
- printf ("\fmessage}\n");
- } else {
- GMimeContentDisposition *disposition;
-
- disposition = g_mime_object_get_content_disposition (meta);
- if (disposition &&
- strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
- {
- printf ("\fattachment}\n");
- }
- else
- {
- printf ("\fpart}\n");
- }
}
+
+ printf ("\f%s}\n", part_type);
}
static void
diff --git a/test/crypto b/test/crypto
index 446a58b..1dbb60a 100755
--- a/test/crypto
+++ b/test/crypto
@@ -159,7 +159,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2000-01-01) (encrypted inbox)
Subject: test encrypted message 001
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: test_suite@notmuchmail.org
-Date: 01 Jan 2000 12:00:00 -0000
+Date: Sat, 01 Jan 2000 12:00:00 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: multipart/encrypted
diff --git a/test/thread-naming b/test/thread-naming
index 2ce9216..942e593 100755
--- a/test/thread-naming
+++ b/test/thread-naming
@@ -71,7 +71,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (unread)
Subject: thread-naming: Initial thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Fri, 05 Jan 2001 15:43:56 -0000
+Date: Fri, 05 Jan 2001 15:43:56 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -85,7 +85,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-06) (inbox unread)
Subject: thread-naming: Older changed subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Sat, 06 Jan 2001 15:43:56 -0000
+Date: Sat, 06 Jan 2001 15:43:56 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -99,7 +99,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-07) (inbox unread)
Subject: thread-naming: Newer changed subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Sun, 07 Jan 2001 15:43:56 -0000
+Date: Sun, 07 Jan 2001 15:43:56 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -113,7 +113,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-08) (unread)
Subject: thread-naming: Final thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Mon, 08 Jan 2001 15:43:56 -0000
+Date: Mon, 08 Jan 2001 15:43:56 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -127,7 +127,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-09) (inbox unread)
Subject: Re: thread-naming: Initial thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Tue, 09 Jan 2001 15:43:45 -0000
+Date: Tue, 09 Jan 2001 15:43:45 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -141,7 +141,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-10) (inbox unread)
Subject: Aw: thread-naming: Initial thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Wed, 10 Jan 2001 15:43:45 -0000
+Date: Wed, 10 Jan 2001 15:43:45 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -155,7 +155,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-11) (inbox unread)
Subject: Vs: thread-naming: Initial thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Thu, 11 Jan 2001 15:43:45 -0000
+Date: Thu, 11 Jan 2001 15:43:45 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -169,7 +169,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-12) (inbox unread)
Subject: Sv: thread-naming: Initial thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Fri, 12 Jan 2001 15:43:45 -0000
+Date: Fri, 12 Jan 2001 15:43:45 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
--
1.7.7.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] show: Simplify new text formatter code
2012-01-26 6:55 ` [PATCH 2/2] show: Simplify new text formatter code Austin Clements
@ 2012-01-30 14:59 ` Tomi Ollila
2012-01-30 23:37 ` Dmitry Kurochkin
1 sibling, 0 replies; 14+ messages in thread
From: Tomi Ollila @ 2012-01-30 14:59 UTC (permalink / raw)
To: Austin Clements, notmuch
On Thu, 26 Jan 2012 01:55:26 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> This makes the text formatter take advantage of the new code
> structure. The previously duplicated header logic is now unified,
> several things that we used to compute repeatedly across different
> callbacks are now computed once, and the code is simpler overall and
> 32% shorter.
>
> Unifying the header logic causes this to format some dates slightly
> differently, so the two affected test cases are updated.
> ---
Looks good, works fine.
Tomi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] show: Convert text format to the new self-recursive style
2012-01-26 6:55 ` [PATCH 1/2] show: Convert text format to the new self-recursive style Austin Clements
@ 2012-01-30 23:26 ` Dmitry Kurochkin
2012-02-04 21:19 ` Austin Clements
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Kurochkin @ 2012-01-30 23:26 UTC (permalink / raw)
To: Austin Clements, notmuch; +Cc: tomi.ollila
On Thu, 26 Jan 2012 01:55:25 -0500, Austin Clements <amdragon@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.
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"))
> - {
> - 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
> -format_part_end_text (GMimeObject *part)
> -{
> - GMimeContentDisposition *disposition;
> -
> - disposition = g_mime_object_get_content_disposition (part);
> - if (disposition &&
> - strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> - {
> - printf ("\fattachment}\n");
> - }
> - else
> - {
> - printf ("\fpart}\n");
> - }
> -}
> -
> -static void
> format_part_start_json (unused (GMimeObject *part), int *part_count)
> {
> printf ("{\"id\": %d", *part_count);
> @@ -844,6 +719,139 @@ format_part_content_raw (GMimeObject *part)
> }
>
> static void
> +format_part_text (const void *ctx, mime_node_t *node,
> + int indent, const notmuch_show_params_t *params)
> +{
> + /* The disposition and content-type metadata are associated with
> + * the envelope for message parts */
> + GMimeObject *meta = node->envelope_part ?
> + GMIME_OBJECT (node->envelope_part) : node->part;
> + GMimeContentType *content_type = g_mime_object_get_content_type (meta);
> + int i;
> +
> + if (node->envelope_file) {
> + notmuch_message_t *message = node->envelope_file;
> + const char *headers[] = {
> + "Subject", "From", "To", "Cc", "Bcc", "Date"
> + };
> + const char *name, *value;
> + unsigned int i;
> +
> + printf ("\fmessage{ ");
> + 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));
> +
> + printf ("\fheader{\n");
> +
> + 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);
> + }
> + printf ("\fheader}\n");
> + } else {
> + GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (meta);
> + const char *cid = g_mime_object_get_content_id (meta);
> +
> + if (disposition &&
> + strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> + {
> + printf ("\fattachment{ ID: %d", node->part_num);
> +
> + } else {
> +
> + printf ("\fpart{ ID: %d", node->part_num);
> + }
> +
> + if (GMIME_IS_PART (node->part))
> + {
> + const char *filename = g_mime_part_get_filename (GMIME_PART (node->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 (node->envelope_part) {
> + GMimeMessage *message = GMIME_MESSAGE (node->part);
> + InternetAddressList *recipients;
> + const char *recipients_string;
> +
> + printf ("\fheader{\n");
> + printf ("Subject: %s\n", g_mime_message_get_subject (message));
> + printf ("From: %s\n", g_mime_message_get_sender (message));
> + recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
> + recipients_string = internet_address_list_to_string (recipients, 0);
> + if (recipients_string)
> + printf ("To: %s\n", recipients_string);
> + recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
> + recipients_string = internet_address_list_to_string (recipients, 0);
> + if (recipients_string)
> + printf ("Cc: %s\n", recipients_string);
> + printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
> + printf ("\fheader}\n");
> + }
> +
> + if (!node->envelope_file) {
> + 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 (node->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));
> + }
> + }
> +
> + if (GMIME_IS_MESSAGE (node->part))
> + printf ("\fbody{\n");
> +
> + for (i = 0; i < node->nchildren; i++)
> + format_part_text (ctx, mime_node_child (node, i), indent, params);
> +
> + if (GMIME_IS_MESSAGE (node->part))
> + printf ("\fbody}\n");
> +
> + if (node->envelope_file) {
> + printf ("\fmessage}\n");
> + } else {
> + GMimeContentDisposition *disposition;
> +
> + disposition = g_mime_object_get_content_disposition (meta);
> + if (disposition &&
> + strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> + {
> + printf ("\fattachment}\n");
> + }
> + else
> + {
> + printf ("\fpart}\n");
> + }
> + }
> +}
> +
> +static void
> show_message (void *ctx,
> const notmuch_show_format_t *format,
> notmuch_message_t *message,
> --
> 1.7.7.3
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] show: Simplify new text formatter code
2012-01-26 6:55 ` [PATCH 2/2] show: Simplify new text formatter code Austin Clements
2012-01-30 14:59 ` Tomi Ollila
@ 2012-01-30 23:37 ` Dmitry Kurochkin
2012-02-04 0:32 ` Austin Clements
1 sibling, 1 reply; 14+ messages in thread
From: Dmitry Kurochkin @ 2012-01-30 23:37 UTC (permalink / raw)
To: Austin Clements, notmuch; +Cc: tomi.ollila
On Thu, 26 Jan 2012 01:55:26 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> This makes the text formatter take advantage of the new code
> structure. The previously duplicated header logic is now unified,
> several things that we used to compute repeatedly across different
> callbacks are now computed once, and the code is simpler overall and
> 32% shorter.
>
> Unifying the header logic causes this to format some dates slightly
> differently, so the two affected test cases are updated.
> ---
Looks good to me. Two minor comments, which can be freely ignored, are
below.
> notmuch-show.c | 88 +++++++++++++--------------------------------------
> test/crypto | 2 +-
> test/thread-naming | 16 +++++-----
> 3 files changed, 32 insertions(+), 74 deletions(-)
>
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 6a890b2..30f6501 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -727,67 +727,48 @@ format_part_text (const void *ctx, mime_node_t *node,
> GMimeObject *meta = node->envelope_part ?
> GMIME_OBJECT (node->envelope_part) : node->part;
> GMimeContentType *content_type = g_mime_object_get_content_type (meta);
> + const notmuch_bool_t leaf = GMIME_IS_PART (node->part);
> + const char *part_type;
> int i;
>
> if (node->envelope_file) {
> notmuch_message_t *message = node->envelope_file;
> - const char *headers[] = {
> - "Subject", "From", "To", "Cc", "Bcc", "Date"
> - };
> - const char *name, *value;
> - unsigned int i;
>
> - printf ("\fmessage{ ");
> - printf ("id:%s depth:%d match:%d filename:%s\n",
> + part_type = "message";
> + printf ("\f%s{ id:%s depth:%d match:%d filename:%s\n",
> + part_type,
> notmuch_message_get_message_id (message),
> indent,
> notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
> notmuch_message_get_filename (message));
> -
> - printf ("\fheader{\n");
> -
> - 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);
> - }
> - printf ("\fheader}\n");
> } else {
> GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (meta);
> const char *cid = g_mime_object_get_content_id (meta);
> + const char *filename = leaf ?
> + g_mime_part_get_filename (GMIME_PART (node->part)) : NULL;
>
> if (disposition &&
> strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> - {
> - printf ("\fattachment{ ID: %d", node->part_num);
> -
> - } else {
> -
> - printf ("\fpart{ ID: %d", node->part_num);
> - }
> -
> - if (GMIME_IS_PART (node->part))
> - {
> - const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
> - if (filename)
> - printf (", Filename: %s", filename);
> - }
> + part_type = "attachment";
> + else
> + part_type = "part";
Others may disagree, but I would write this using the (? :) operator.
Not important, feel free to leave it as is.
>
> + printf ("\f%s{ ID: %d", part_type, node->part_num);
> + 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 (node->envelope_part) {
> + if (GMIME_IS_MESSAGE (node->part)) {
> GMimeMessage *message = GMIME_MESSAGE (node->part);
> InternetAddressList *recipients;
> const char *recipients_string;
>
> printf ("\fheader{\n");
> + if (node->envelope_file)
> + printf ("%s\n", _get_one_line_summary (ctx, node->envelope_file));
> printf ("Subject: %s\n", g_mime_message_get_subject (message));
> printf ("From: %s\n", g_mime_message_get_sender (message));
> recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
> @@ -800,9 +781,11 @@ format_part_text (const void *ctx, mime_node_t *node,
> printf ("Cc: %s\n", recipients_string);
> printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
> printf ("\fheader}\n");
> +
> + printf ("\fbody{\n");
> }
>
> - if (!node->envelope_file) {
> + if (leaf) {
> if (g_mime_content_type_is_type (content_type, "text", "*") &&
> !g_mime_content_type_is_type (content_type, "text", "html"))
> {
> @@ -810,45 +793,20 @@ format_part_text (const void *ctx, mime_node_t *node,
> g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
> show_text_part_content (node->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
> - {
> + } else {
> printf ("Non-text part: %s\n",
> g_mime_content_type_to_string (content_type));
> }
> }
>
> - if (GMIME_IS_MESSAGE (node->part))
> - printf ("\fbody{\n");
> -
> for (i = 0; i < node->nchildren; i++)
> format_part_text (ctx, mime_node_child (node, i), indent, params);
>
> - if (GMIME_IS_MESSAGE (node->part))
> + if (GMIME_IS_MESSAGE (node->part)) {
Since the if body is one line, I would prefer to remove the closing
bracket rather than adding the opening one.
Regards,
Dmitry
> printf ("\fbody}\n");
> -
> - if (node->envelope_file) {
> - printf ("\fmessage}\n");
> - } else {
> - GMimeContentDisposition *disposition;
> -
> - disposition = g_mime_object_get_content_disposition (meta);
> - if (disposition &&
> - strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> - {
> - printf ("\fattachment}\n");
> - }
> - else
> - {
> - printf ("\fpart}\n");
> - }
> }
> +
> + printf ("\f%s}\n", part_type);
> }
>
> static void
> diff --git a/test/crypto b/test/crypto
> index 446a58b..1dbb60a 100755
> --- a/test/crypto
> +++ b/test/crypto
> @@ -159,7 +159,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2000-01-01) (encrypted inbox)
> Subject: test encrypted message 001
> From: Notmuch Test Suite <test_suite@notmuchmail.org>
> To: test_suite@notmuchmail.org
> -Date: 01 Jan 2000 12:00:00 -0000
> +Date: Sat, 01 Jan 2000 12:00:00 +0000
> \fheader}
> \fbody{
> \fpart{ ID: 1, Content-type: multipart/encrypted
> diff --git a/test/thread-naming b/test/thread-naming
> index 2ce9216..942e593 100755
> --- a/test/thread-naming
> +++ b/test/thread-naming
> @@ -71,7 +71,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (unread)
> Subject: thread-naming: Initial thread subject
> From: Notmuch Test Suite <test_suite@notmuchmail.org>
> To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Fri, 05 Jan 2001 15:43:56 -0000
> +Date: Fri, 05 Jan 2001 15:43:56 +0000
> \fheader}
> \fbody{
> \fpart{ ID: 1, Content-type: text/plain
> @@ -85,7 +85,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-06) (inbox unread)
> Subject: thread-naming: Older changed subject
> From: Notmuch Test Suite <test_suite@notmuchmail.org>
> To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Sat, 06 Jan 2001 15:43:56 -0000
> +Date: Sat, 06 Jan 2001 15:43:56 +0000
> \fheader}
> \fbody{
> \fpart{ ID: 1, Content-type: text/plain
> @@ -99,7 +99,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-07) (inbox unread)
> Subject: thread-naming: Newer changed subject
> From: Notmuch Test Suite <test_suite@notmuchmail.org>
> To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Sun, 07 Jan 2001 15:43:56 -0000
> +Date: Sun, 07 Jan 2001 15:43:56 +0000
> \fheader}
> \fbody{
> \fpart{ ID: 1, Content-type: text/plain
> @@ -113,7 +113,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-08) (unread)
> Subject: thread-naming: Final thread subject
> From: Notmuch Test Suite <test_suite@notmuchmail.org>
> To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Mon, 08 Jan 2001 15:43:56 -0000
> +Date: Mon, 08 Jan 2001 15:43:56 +0000
> \fheader}
> \fbody{
> \fpart{ ID: 1, Content-type: text/plain
> @@ -127,7 +127,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-09) (inbox unread)
> Subject: Re: thread-naming: Initial thread subject
> From: Notmuch Test Suite <test_suite@notmuchmail.org>
> To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Tue, 09 Jan 2001 15:43:45 -0000
> +Date: Tue, 09 Jan 2001 15:43:45 +0000
> \fheader}
> \fbody{
> \fpart{ ID: 1, Content-type: text/plain
> @@ -141,7 +141,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-10) (inbox unread)
> Subject: Aw: thread-naming: Initial thread subject
> From: Notmuch Test Suite <test_suite@notmuchmail.org>
> To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Wed, 10 Jan 2001 15:43:45 -0000
> +Date: Wed, 10 Jan 2001 15:43:45 +0000
> \fheader}
> \fbody{
> \fpart{ ID: 1, Content-type: text/plain
> @@ -155,7 +155,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-11) (inbox unread)
> Subject: Vs: thread-naming: Initial thread subject
> From: Notmuch Test Suite <test_suite@notmuchmail.org>
> To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Thu, 11 Jan 2001 15:43:45 -0000
> +Date: Thu, 11 Jan 2001 15:43:45 +0000
> \fheader}
> \fbody{
> \fpart{ ID: 1, Content-type: text/plain
> @@ -169,7 +169,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-12) (inbox unread)
> Subject: Sv: thread-naming: Initial thread subject
> From: Notmuch Test Suite <test_suite@notmuchmail.org>
> To: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Date: Fri, 12 Jan 2001 15:43:45 -0000
> +Date: Fri, 12 Jan 2001 15:43:45 +0000
> \fheader}
> \fbody{
> \fpart{ ID: 1, Content-type: text/plain
> --
> 1.7.7.3
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] show: Simplify new text formatter code
2012-01-30 23:37 ` Dmitry Kurochkin
@ 2012-02-04 0:32 ` Austin Clements
0 siblings, 0 replies; 14+ messages in thread
From: Austin Clements @ 2012-02-04 0:32 UTC (permalink / raw)
To: Dmitry Kurochkin; +Cc: tomi.ollila, notmuch
Quoth Dmitry Kurochkin on Jan 31 at 3:37 am:
> On Thu, 26 Jan 2012 01:55:26 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > This makes the text formatter take advantage of the new code
> > structure. The previously duplicated header logic is now unified,
> > several things that we used to compute repeatedly across different
> > callbacks are now computed once, and the code is simpler overall and
> > 32% shorter.
> >
> > Unifying the header logic causes this to format some dates slightly
> > differently, so the two affected test cases are updated.
> > ---
>
> Looks good to me. Two minor comments, which can be freely ignored, are
> below.
>
> > notmuch-show.c | 88 +++++++++++++--------------------------------------
> > test/crypto | 2 +-
> > test/thread-naming | 16 +++++-----
> > 3 files changed, 32 insertions(+), 74 deletions(-)
> >
> > diff --git a/notmuch-show.c b/notmuch-show.c
> > index 6a890b2..30f6501 100644
> > --- a/notmuch-show.c
> > +++ b/notmuch-show.c
> > @@ -727,67 +727,48 @@ format_part_text (const void *ctx, mime_node_t *node,
> > GMimeObject *meta = node->envelope_part ?
> > GMIME_OBJECT (node->envelope_part) : node->part;
> > GMimeContentType *content_type = g_mime_object_get_content_type (meta);
> > + const notmuch_bool_t leaf = GMIME_IS_PART (node->part);
> > + const char *part_type;
> > int i;
> >
> > if (node->envelope_file) {
> > notmuch_message_t *message = node->envelope_file;
> > - const char *headers[] = {
> > - "Subject", "From", "To", "Cc", "Bcc", "Date"
> > - };
> > - const char *name, *value;
> > - unsigned int i;
> >
> > - printf ("\fmessage{ ");
> > - printf ("id:%s depth:%d match:%d filename:%s\n",
> > + part_type = "message";
> > + printf ("\f%s{ id:%s depth:%d match:%d filename:%s\n",
> > + part_type,
> > notmuch_message_get_message_id (message),
> > indent,
> > notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
> > notmuch_message_get_filename (message));
> > -
> > - printf ("\fheader{\n");
> > -
> > - 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);
> > - }
> > - printf ("\fheader}\n");
> > } else {
> > GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (meta);
> > const char *cid = g_mime_object_get_content_id (meta);
> > + const char *filename = leaf ?
> > + g_mime_part_get_filename (GMIME_PART (node->part)) : NULL;
> >
> > if (disposition &&
> > strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> > - {
> > - printf ("\fattachment{ ID: %d", node->part_num);
> > -
> > - } else {
> > -
> > - printf ("\fpart{ ID: %d", node->part_num);
> > - }
> > -
> > - if (GMIME_IS_PART (node->part))
> > - {
> > - const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
> > - if (filename)
> > - printf (", Filename: %s", filename);
> > - }
> > + part_type = "attachment";
> > + else
> > + part_type = "part";
>
> Others may disagree, but I would write this using the (? :) operator.
> Not important, feel free to leave it as is.
Normally I would, too, but when the condition is that long, I find it
makes it more obtuse.
> >
> > + printf ("\f%s{ ID: %d", part_type, node->part_num);
> > + 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 (node->envelope_part) {
> > + if (GMIME_IS_MESSAGE (node->part)) {
> > GMimeMessage *message = GMIME_MESSAGE (node->part);
> > InternetAddressList *recipients;
> > const char *recipients_string;
> >
> > printf ("\fheader{\n");
> > + if (node->envelope_file)
> > + printf ("%s\n", _get_one_line_summary (ctx, node->envelope_file));
> > printf ("Subject: %s\n", g_mime_message_get_subject (message));
> > printf ("From: %s\n", g_mime_message_get_sender (message));
> > recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
> > @@ -800,9 +781,11 @@ format_part_text (const void *ctx, mime_node_t *node,
> > printf ("Cc: %s\n", recipients_string);
> > printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
> > printf ("\fheader}\n");
> > +
> > + printf ("\fbody{\n");
> > }
> >
> > - if (!node->envelope_file) {
> > + if (leaf) {
> > if (g_mime_content_type_is_type (content_type, "text", "*") &&
> > !g_mime_content_type_is_type (content_type, "text", "html"))
> > {
> > @@ -810,45 +793,20 @@ format_part_text (const void *ctx, mime_node_t *node,
> > g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
> > show_text_part_content (node->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
> > - {
> > + } else {
> > printf ("Non-text part: %s\n",
> > g_mime_content_type_to_string (content_type));
> > }
> > }
> >
> > - if (GMIME_IS_MESSAGE (node->part))
> > - printf ("\fbody{\n");
> > -
> > for (i = 0; i < node->nchildren; i++)
> > format_part_text (ctx, mime_node_child (node, i), indent, params);
> >
> > - if (GMIME_IS_MESSAGE (node->part))
> > + if (GMIME_IS_MESSAGE (node->part)) {
>
> Since the if body is one line, I would prefer to remove the closing
> bracket rather than adding the opening one.
Oops. That must have been a rebasing mixup. Fixed.
> Regards,
> Dmitry
>
> > printf ("\fbody}\n");
> > -
> > - if (node->envelope_file) {
> > - printf ("\fmessage}\n");
> > - } else {
> > - GMimeContentDisposition *disposition;
> > -
> > - disposition = g_mime_object_get_content_disposition (meta);
> > - if (disposition &&
> > - strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> > - {
> > - printf ("\fattachment}\n");
> > - }
> > - else
> > - {
> > - printf ("\fpart}\n");
> > - }
> > }
> > +
> > + printf ("\f%s}\n", part_type);
> > }
> >
> > static void
> > diff --git a/test/crypto b/test/crypto
> > index 446a58b..1dbb60a 100755
> > --- a/test/crypto
> > +++ b/test/crypto
> > @@ -159,7 +159,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2000-01-01) (encrypted inbox)
> > Subject: test encrypted message 001
> > From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > To: test_suite@notmuchmail.org
> > -Date: 01 Jan 2000 12:00:00 -0000
> > +Date: Sat, 01 Jan 2000 12:00:00 +0000
> > \fheader}
> > \fbody{
> > \fpart{ ID: 1, Content-type: multipart/encrypted
> > diff --git a/test/thread-naming b/test/thread-naming
> > index 2ce9216..942e593 100755
> > --- a/test/thread-naming
> > +++ b/test/thread-naming
> > @@ -71,7 +71,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (unread)
> > Subject: thread-naming: Initial thread subject
> > From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Fri, 05 Jan 2001 15:43:56 -0000
> > +Date: Fri, 05 Jan 2001 15:43:56 +0000
> > \fheader}
> > \fbody{
> > \fpart{ ID: 1, Content-type: text/plain
> > @@ -85,7 +85,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-06) (inbox unread)
> > Subject: thread-naming: Older changed subject
> > From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Sat, 06 Jan 2001 15:43:56 -0000
> > +Date: Sat, 06 Jan 2001 15:43:56 +0000
> > \fheader}
> > \fbody{
> > \fpart{ ID: 1, Content-type: text/plain
> > @@ -99,7 +99,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-07) (inbox unread)
> > Subject: thread-naming: Newer changed subject
> > From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Sun, 07 Jan 2001 15:43:56 -0000
> > +Date: Sun, 07 Jan 2001 15:43:56 +0000
> > \fheader}
> > \fbody{
> > \fpart{ ID: 1, Content-type: text/plain
> > @@ -113,7 +113,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-08) (unread)
> > Subject: thread-naming: Final thread subject
> > From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Mon, 08 Jan 2001 15:43:56 -0000
> > +Date: Mon, 08 Jan 2001 15:43:56 +0000
> > \fheader}
> > \fbody{
> > \fpart{ ID: 1, Content-type: text/plain
> > @@ -127,7 +127,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-09) (inbox unread)
> > Subject: Re: thread-naming: Initial thread subject
> > From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Tue, 09 Jan 2001 15:43:45 -0000
> > +Date: Tue, 09 Jan 2001 15:43:45 +0000
> > \fheader}
> > \fbody{
> > \fpart{ ID: 1, Content-type: text/plain
> > @@ -141,7 +141,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-10) (inbox unread)
> > Subject: Aw: thread-naming: Initial thread subject
> > From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Wed, 10 Jan 2001 15:43:45 -0000
> > +Date: Wed, 10 Jan 2001 15:43:45 +0000
> > \fheader}
> > \fbody{
> > \fpart{ ID: 1, Content-type: text/plain
> > @@ -155,7 +155,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-11) (inbox unread)
> > Subject: Vs: thread-naming: Initial thread subject
> > From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Thu, 11 Jan 2001 15:43:45 -0000
> > +Date: Thu, 11 Jan 2001 15:43:45 +0000
> > \fheader}
> > \fbody{
> > \fpart{ ID: 1, Content-type: text/plain
> > @@ -169,7 +169,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-12) (inbox unread)
> > Subject: Sv: thread-naming: Initial thread subject
> > From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > To: Notmuch Test Suite <test_suite@notmuchmail.org>
> > -Date: Fri, 12 Jan 2001 15:43:45 -0000
> > +Date: Fri, 12 Jan 2001 15:43:45 +0000
> > \fheader}
> > \fbody{
> > \fpart{ ID: 1, Content-type: text/plain
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] show: Convert text format to the new self-recursive style
2012-01-30 23:26 ` Dmitry Kurochkin
@ 2012-02-04 21:19 ` Austin Clements
0 siblings, 0 replies; 14+ messages in thread
From: Austin Clements @ 2012-02-04 21:19 UTC (permalink / raw)
To: Dmitry Kurochkin; +Cc: tomi.ollila, notmuch
Quoth Dmitry Kurochkin on Jan 31 at 3:26 am:
> On Thu, 26 Jan 2012 01:55:25 -0500, Austin Clements <amdragon@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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 0/2] Rewrite text show format
2012-01-26 6:55 [PATCH 0/2] Rewrite text show format Austin Clements
2012-01-26 6:55 ` [PATCH 1/2] show: Convert text format to the new self-recursive style Austin Clements
2012-01-26 6:55 ` [PATCH 2/2] show: Simplify new text formatter code Austin Clements
@ 2012-02-04 21:24 ` Austin Clements
2012-02-04 21:24 ` [PATCH v2 1/2] show: Convert text format to the new self-recursive style Austin Clements
` (3 more replies)
2012-02-14 14:49 ` [PATCH " Adam Wolfe Gordon
3 siblings, 4 replies; 14+ messages in thread
From: Austin Clements @ 2012-02-04 21:24 UTC (permalink / raw)
To: notmuch
v2
* Remove unnecessary braces (id:87lioooj7m.fsf@gmail.com)
* Trivial rebase against master
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/2] show: Convert text format to the new self-recursive style
2012-02-04 21:24 ` [PATCH v2 0/2] Rewrite text show format Austin Clements
@ 2012-02-04 21:24 ` Austin Clements
2012-02-04 21:24 ` [PATCH v2 2/2] show: Simplify new text formatter code Austin Clements
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Austin Clements @ 2012-02-04 21:24 UTC (permalink / raw)
To: notmuch
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
-format_part_end_text (GMimeObject *part)
-{
- GMimeContentDisposition *disposition;
-
- disposition = g_mime_object_get_content_disposition (part);
- if (disposition &&
- strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
- {
- printf ("\fattachment}\n");
- }
- else
- {
- printf ("\fpart}\n");
- }
-}
-
-static void
format_part_start_json (unused (GMimeObject *part), int *part_count)
{
printf ("{\"id\": %d", *part_count);
@@ -844,6 +719,139 @@ format_part_content_raw (GMimeObject *part)
}
static void
+format_part_text (const void *ctx, mime_node_t *node,
+ int indent, const notmuch_show_params_t *params)
+{
+ /* The disposition and content-type metadata are associated with
+ * the envelope for message parts */
+ GMimeObject *meta = node->envelope_part ?
+ GMIME_OBJECT (node->envelope_part) : node->part;
+ GMimeContentType *content_type = g_mime_object_get_content_type (meta);
+ int i;
+
+ if (node->envelope_file) {
+ notmuch_message_t *message = node->envelope_file;
+ const char *headers[] = {
+ "Subject", "From", "To", "Cc", "Bcc", "Date"
+ };
+ const char *name, *value;
+ unsigned int i;
+
+ printf ("\fmessage{ ");
+ 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));
+
+ printf ("\fheader{\n");
+
+ 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);
+ }
+ printf ("\fheader}\n");
+ } else {
+ GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (meta);
+ const char *cid = g_mime_object_get_content_id (meta);
+
+ if (disposition &&
+ strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
+ {
+ printf ("\fattachment{ ID: %d", node->part_num);
+
+ } else {
+
+ printf ("\fpart{ ID: %d", node->part_num);
+ }
+
+ if (GMIME_IS_PART (node->part))
+ {
+ const char *filename = g_mime_part_get_filename (GMIME_PART (node->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 (node->envelope_part) {
+ GMimeMessage *message = GMIME_MESSAGE (node->part);
+ InternetAddressList *recipients;
+ const char *recipients_string;
+
+ printf ("\fheader{\n");
+ printf ("Subject: %s\n", g_mime_message_get_subject (message));
+ printf ("From: %s\n", g_mime_message_get_sender (message));
+ recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
+ recipients_string = internet_address_list_to_string (recipients, 0);
+ if (recipients_string)
+ printf ("To: %s\n", recipients_string);
+ recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
+ recipients_string = internet_address_list_to_string (recipients, 0);
+ if (recipients_string)
+ printf ("Cc: %s\n", recipients_string);
+ printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
+ printf ("\fheader}\n");
+ }
+
+ if (!node->envelope_file) {
+ 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 (node->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));
+ }
+ }
+
+ if (GMIME_IS_MESSAGE (node->part))
+ printf ("\fbody{\n");
+
+ for (i = 0; i < node->nchildren; i++)
+ format_part_text (ctx, mime_node_child (node, i), indent, params);
+
+ if (GMIME_IS_MESSAGE (node->part))
+ printf ("\fbody}\n");
+
+ if (node->envelope_file) {
+ printf ("\fmessage}\n");
+ } else {
+ GMimeContentDisposition *disposition;
+
+ disposition = g_mime_object_get_content_disposition (meta);
+ if (disposition &&
+ strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
+ {
+ printf ("\fattachment}\n");
+ }
+ else
+ {
+ printf ("\fpart}\n");
+ }
+ }
+}
+
+static void
show_message (void *ctx,
const notmuch_show_format_t *format,
notmuch_message_t *message,
--
1.7.7.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/2] show: Simplify new text formatter code
2012-02-04 21:24 ` [PATCH v2 0/2] Rewrite text show format Austin Clements
2012-02-04 21:24 ` [PATCH v2 1/2] show: Convert text format to the new self-recursive style Austin Clements
@ 2012-02-04 21:24 ` Austin Clements
2012-02-05 18:36 ` [PATCH v2 0/2] Rewrite text show format Dmitry Kurochkin
2012-02-12 17:33 ` David Bremner
3 siblings, 0 replies; 14+ messages in thread
From: Austin Clements @ 2012-02-04 21:24 UTC (permalink / raw)
To: notmuch
This makes the text formatter take advantage of the new code
structure. The previously duplicated header logic is now unified,
several things that we used to compute repeatedly across different
callbacks are now computed once, and the code is simpler overall and
32% shorter.
Unifying the header logic causes this to format some dates slightly
differently, so the two affected test cases are updated.
---
notmuch-show.c | 85 +++++++++++++---------------------------------------
test/crypto | 2 +-
test/thread-naming | 16 +++++-----
3 files changed, 30 insertions(+), 73 deletions(-)
diff --git a/notmuch-show.c b/notmuch-show.c
index 6a890b2..816e0f8 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -727,67 +727,48 @@ format_part_text (const void *ctx, mime_node_t *node,
GMimeObject *meta = node->envelope_part ?
GMIME_OBJECT (node->envelope_part) : node->part;
GMimeContentType *content_type = g_mime_object_get_content_type (meta);
+ const notmuch_bool_t leaf = GMIME_IS_PART (node->part);
+ const char *part_type;
int i;
if (node->envelope_file) {
notmuch_message_t *message = node->envelope_file;
- const char *headers[] = {
- "Subject", "From", "To", "Cc", "Bcc", "Date"
- };
- const char *name, *value;
- unsigned int i;
- printf ("\fmessage{ ");
- printf ("id:%s depth:%d match:%d filename:%s\n",
+ part_type = "message";
+ printf ("\f%s{ id:%s depth:%d match:%d filename:%s\n",
+ part_type,
notmuch_message_get_message_id (message),
indent,
notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
notmuch_message_get_filename (message));
-
- printf ("\fheader{\n");
-
- 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);
- }
- printf ("\fheader}\n");
} else {
GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (meta);
const char *cid = g_mime_object_get_content_id (meta);
+ const char *filename = leaf ?
+ g_mime_part_get_filename (GMIME_PART (node->part)) : NULL;
if (disposition &&
strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
- {
- printf ("\fattachment{ ID: %d", node->part_num);
-
- } else {
-
- printf ("\fpart{ ID: %d", node->part_num);
- }
-
- if (GMIME_IS_PART (node->part))
- {
- const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
- if (filename)
- printf (", Filename: %s", filename);
- }
+ part_type = "attachment";
+ else
+ part_type = "part";
+ printf ("\f%s{ ID: %d", part_type, node->part_num);
+ 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 (node->envelope_part) {
+ if (GMIME_IS_MESSAGE (node->part)) {
GMimeMessage *message = GMIME_MESSAGE (node->part);
InternetAddressList *recipients;
const char *recipients_string;
printf ("\fheader{\n");
+ if (node->envelope_file)
+ printf ("%s\n", _get_one_line_summary (ctx, node->envelope_file));
printf ("Subject: %s\n", g_mime_message_get_subject (message));
printf ("From: %s\n", g_mime_message_get_sender (message));
recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
@@ -800,9 +781,11 @@ format_part_text (const void *ctx, mime_node_t *node,
printf ("Cc: %s\n", recipients_string);
printf ("Date: %s\n", g_mime_message_get_date_as_string (message));
printf ("\fheader}\n");
+
+ printf ("\fbody{\n");
}
- if (!node->envelope_file) {
+ if (leaf) {
if (g_mime_content_type_is_type (content_type, "text", "*") &&
!g_mime_content_type_is_type (content_type, "text", "html"))
{
@@ -810,45 +793,19 @@ format_part_text (const void *ctx, mime_node_t *node,
g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
show_text_part_content (node->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
- {
+ } else {
printf ("Non-text part: %s\n",
g_mime_content_type_to_string (content_type));
}
}
- if (GMIME_IS_MESSAGE (node->part))
- printf ("\fbody{\n");
-
for (i = 0; i < node->nchildren; i++)
format_part_text (ctx, mime_node_child (node, i), indent, params);
if (GMIME_IS_MESSAGE (node->part))
printf ("\fbody}\n");
- if (node->envelope_file) {
- printf ("\fmessage}\n");
- } else {
- GMimeContentDisposition *disposition;
-
- disposition = g_mime_object_get_content_disposition (meta);
- if (disposition &&
- strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
- {
- printf ("\fattachment}\n");
- }
- else
- {
- printf ("\fpart}\n");
- }
- }
+ printf ("\f%s}\n", part_type);
}
static void
diff --git a/test/crypto b/test/crypto
index 446a58b..1dbb60a 100755
--- a/test/crypto
+++ b/test/crypto
@@ -159,7 +159,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2000-01-01) (encrypted inbox)
Subject: test encrypted message 001
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: test_suite@notmuchmail.org
-Date: 01 Jan 2000 12:00:00 -0000
+Date: Sat, 01 Jan 2000 12:00:00 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: multipart/encrypted
diff --git a/test/thread-naming b/test/thread-naming
index 2ce9216..942e593 100755
--- a/test/thread-naming
+++ b/test/thread-naming
@@ -71,7 +71,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-05) (unread)
Subject: thread-naming: Initial thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Fri, 05 Jan 2001 15:43:56 -0000
+Date: Fri, 05 Jan 2001 15:43:56 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -85,7 +85,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-06) (inbox unread)
Subject: thread-naming: Older changed subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Sat, 06 Jan 2001 15:43:56 -0000
+Date: Sat, 06 Jan 2001 15:43:56 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -99,7 +99,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-07) (inbox unread)
Subject: thread-naming: Newer changed subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Sun, 07 Jan 2001 15:43:56 -0000
+Date: Sun, 07 Jan 2001 15:43:56 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -113,7 +113,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-08) (unread)
Subject: thread-naming: Final thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Mon, 08 Jan 2001 15:43:56 -0000
+Date: Mon, 08 Jan 2001 15:43:56 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -127,7 +127,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-09) (inbox unread)
Subject: Re: thread-naming: Initial thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Tue, 09 Jan 2001 15:43:45 -0000
+Date: Tue, 09 Jan 2001 15:43:45 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -141,7 +141,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-10) (inbox unread)
Subject: Aw: thread-naming: Initial thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Wed, 10 Jan 2001 15:43:45 -0000
+Date: Wed, 10 Jan 2001 15:43:45 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -155,7 +155,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-11) (inbox unread)
Subject: Vs: thread-naming: Initial thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Thu, 11 Jan 2001 15:43:45 -0000
+Date: Thu, 11 Jan 2001 15:43:45 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
@@ -169,7 +169,7 @@ Notmuch Test Suite <test_suite@notmuchmail.org> (2001-01-12) (inbox unread)
Subject: Sv: thread-naming: Initial thread subject
From: Notmuch Test Suite <test_suite@notmuchmail.org>
To: Notmuch Test Suite <test_suite@notmuchmail.org>
-Date: Fri, 12 Jan 2001 15:43:45 -0000
+Date: Fri, 12 Jan 2001 15:43:45 +0000
\fheader}
\fbody{
\fpart{ ID: 1, Content-type: text/plain
--
1.7.7.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/2] Rewrite text show format
2012-02-04 21:24 ` [PATCH v2 0/2] Rewrite text show format Austin Clements
2012-02-04 21:24 ` [PATCH v2 1/2] show: Convert text format to the new self-recursive style Austin Clements
2012-02-04 21:24 ` [PATCH v2 2/2] show: Simplify new text formatter code Austin Clements
@ 2012-02-05 18:36 ` Dmitry Kurochkin
2012-02-12 17:33 ` David Bremner
3 siblings, 0 replies; 14+ messages in thread
From: Dmitry Kurochkin @ 2012-02-05 18:36 UTC (permalink / raw)
To: David Bremner, Austin Clements, notmuch
On Sat, 4 Feb 2012 16:24:24 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> v2
>
> * Remove unnecessary braces (id:87lioooj7m.fsf@gmail.com)
>
> * Trivial rebase against master
>
Based on the change log and IRC discussion, I think this series does not
need another round of reviews. So it looks like ready for master and I
removed needs-review tag.
Regards,
Dmitry
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/2] Rewrite text show format
2012-02-04 21:24 ` [PATCH v2 0/2] Rewrite text show format Austin Clements
` (2 preceding siblings ...)
2012-02-05 18:36 ` [PATCH v2 0/2] Rewrite text show format Dmitry Kurochkin
@ 2012-02-12 17:33 ` David Bremner
3 siblings, 0 replies; 14+ messages in thread
From: David Bremner @ 2012-02-12 17:33 UTC (permalink / raw)
To: Austin Clements, notmuch
On Sat, 4 Feb 2012 16:24:24 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> v2
>
> * Remove unnecessary braces (id:87lioooj7m.fsf@gmail.com)
>
> * Trivial rebase against master
Pushed,
d
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] Rewrite text show format
2012-01-26 6:55 [PATCH 0/2] Rewrite text show format Austin Clements
` (2 preceding siblings ...)
2012-02-04 21:24 ` [PATCH v2 0/2] Rewrite text show format Austin Clements
@ 2012-02-14 14:49 ` Adam Wolfe Gordon
3 siblings, 0 replies; 14+ messages in thread
From: Adam Wolfe Gordon @ 2012-02-14 14:49 UTC (permalink / raw)
To: Austin Clements; +Cc: tomi.ollila, notmuch
Hi Austin,
On Wed, Jan 25, 2012 at 23:55, Austin Clements <amdragon@mit.edu> wrote:
> And now the real fun begins. This series translates the text
> formatter into the new format style in two steps: the first patch is a
> big diff but just shuffles code and the second actually takes
> advantage of the new structure.
Thanks for this work. I'm now rewriting the JSON format to use the new
structure, which will help simplify my reply patches.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-02-14 14:49 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-26 6:55 [PATCH 0/2] Rewrite text show format Austin Clements
2012-01-26 6:55 ` [PATCH 1/2] show: Convert text format to the new self-recursive style Austin Clements
2012-01-30 23:26 ` Dmitry Kurochkin
2012-02-04 21:19 ` Austin Clements
2012-01-26 6:55 ` [PATCH 2/2] show: Simplify new text formatter code Austin Clements
2012-01-30 14:59 ` Tomi Ollila
2012-01-30 23:37 ` Dmitry Kurochkin
2012-02-04 0:32 ` Austin Clements
2012-02-04 21:24 ` [PATCH v2 0/2] Rewrite text show format Austin Clements
2012-02-04 21:24 ` [PATCH v2 1/2] show: Convert text format to the new self-recursive style Austin Clements
2012-02-04 21:24 ` [PATCH v2 2/2] show: Simplify new text formatter code Austin Clements
2012-02-05 18:36 ` [PATCH v2 0/2] Rewrite text show format Dmitry Kurochkin
2012-02-12 17:33 ` David Bremner
2012-02-14 14:49 ` [PATCH " Adam Wolfe Gordon
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).