notmuch

[PATCH 0/2] Rewrite text show format

Classic

List

Threaded

14 messages Options

Austin Clements

[PATCH 0/2] Rewrite text show format

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:[hidden email] .

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Remove Ads

Austin Clements

[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
-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

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Austin Clements

[PATCH 2/2] show: Simplify new text formatter code

In reply to this post by Austin Clements

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 <[hidden email]> (2000-01-01) (encrypted inbox)
Subject: test encrypted message 001
From: Notmuch Test Suite <[hidden email]>
To: [hidden email]
-Date: 01 Jan 2000 12:00:00 -0000
+Date: Sat, 01 Jan 2000 12:00:00 +0000
header}
body{
part{ 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 <[hidden email]> (2001-01-05) (unread)
Subject: thread-naming: Initial thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Fri, 05 Jan 2001 15:43:56 -0000
+Date: Fri, 05 Jan 2001 15:43:56 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -85,7 +85,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-06) (inbox unread)
Subject: thread-naming: Older changed subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Sat, 06 Jan 2001 15:43:56 -0000
+Date: Sat, 06 Jan 2001 15:43:56 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -99,7 +99,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-07) (inbox unread)
Subject: thread-naming: Newer changed subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Sun, 07 Jan 2001 15:43:56 -0000
+Date: Sun, 07 Jan 2001 15:43:56 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -113,7 +113,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-08) (unread)
Subject: thread-naming: Final thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Mon, 08 Jan 2001 15:43:56 -0000
+Date: Mon, 08 Jan 2001 15:43:56 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -127,7 +127,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-09) (inbox unread)
Subject: Re: thread-naming: Initial thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Tue, 09 Jan 2001 15:43:45 -0000
+Date: Tue, 09 Jan 2001 15:43:45 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -141,7 +141,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-10) (inbox unread)
Subject: Aw: thread-naming: Initial thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Wed, 10 Jan 2001 15:43:45 -0000
+Date: Wed, 10 Jan 2001 15:43:45 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -155,7 +155,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-11) (inbox unread)
Subject: Vs: thread-naming: Initial thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Thu, 11 Jan 2001 15:43:45 -0000
+Date: Thu, 11 Jan 2001 15:43:45 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -169,7 +169,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-12) (inbox unread)
Subject: Sv: thread-naming: Initial thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Fri, 12 Jan 2001 15:43:45 -0000
+Date: Fri, 12 Jan 2001 15:43:45 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Tomi Ollila-2

Re: [PATCH 2/2] show: Simplify new text formatter code

On Thu, 26 Jan 2012 01:55:26 -0500, Austin Clements <[hidden email]> 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
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Dmitry Kurochkin

Re: [PATCH 1/2] show: Convert text format to the new self-recursive style

In reply to this post by Austin Clements

On Thu, 26 Jan 2012 01:55:25 -0500, Austin Clements <[hidden email]> 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
>

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Dmitry Kurochkin

Re: [PATCH 2/2] show: Simplify new text formatter code

In reply to this post by Austin Clements

On Thu, 26 Jan 2012 01:55:26 -0500, Austin Clements <[hidden email]> 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 <[hidden email]> (2000-01-01) (encrypted inbox)
> Subject: test encrypted message 001
> From: Notmuch Test Suite <[hidden email]>
> To: [hidden email]
> -Date: 01 Jan 2000 12:00:00 -0000
> +Date: Sat, 01 Jan 2000 12:00:00 +0000
> header}
> body{
> part{ 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 <[hidden email]> (2001-01-05) (unread)
> Subject: thread-naming: Initial thread subject
> From: Notmuch Test Suite <[hidden email]>
> To: Notmuch Test Suite <[hidden email]>
> -Date: Fri, 05 Jan 2001 15:43:56 -0000
> +Date: Fri, 05 Jan 2001 15:43:56 +0000
> header}
> body{
> part{ ID: 1, Content-type: text/plain
> @@ -85,7 +85,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-06) (inbox unread)
> Subject: thread-naming: Older changed subject
> From: Notmuch Test Suite <[hidden email]>
> To: Notmuch Test Suite <[hidden email]>
> -Date: Sat, 06 Jan 2001 15:43:56 -0000
> +Date: Sat, 06 Jan 2001 15:43:56 +0000
> header}
> body{
> part{ ID: 1, Content-type: text/plain
> @@ -99,7 +99,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-07) (inbox unread)
> Subject: thread-naming: Newer changed subject
> From: Notmuch Test Suite <[hidden email]>
> To: Notmuch Test Suite <[hidden email]>
> -Date: Sun, 07 Jan 2001 15:43:56 -0000
> +Date: Sun, 07 Jan 2001 15:43:56 +0000
> header}
> body{
> part{ ID: 1, Content-type: text/plain
> @@ -113,7 +113,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-08) (unread)
> Subject: thread-naming: Final thread subject
> From: Notmuch Test Suite <[hidden email]>
> To: Notmuch Test Suite <[hidden email]>
> -Date: Mon, 08 Jan 2001 15:43:56 -0000
> +Date: Mon, 08 Jan 2001 15:43:56 +0000
> header}
> body{
> part{ ID: 1, Content-type: text/plain
> @@ -127,7 +127,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-09) (inbox unread)
> Subject: Re: thread-naming: Initial thread subject
> From: Notmuch Test Suite <[hidden email]>
> To: Notmuch Test Suite <[hidden email]>
> -Date: Tue, 09 Jan 2001 15:43:45 -0000
> +Date: Tue, 09 Jan 2001 15:43:45 +0000
> header}
> body{
> part{ ID: 1, Content-type: text/plain
> @@ -141,7 +141,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-10) (inbox unread)
> Subject: Aw: thread-naming: Initial thread subject
> From: Notmuch Test Suite <[hidden email]>
> To: Notmuch Test Suite <[hidden email]>
> -Date: Wed, 10 Jan 2001 15:43:45 -0000
> +Date: Wed, 10 Jan 2001 15:43:45 +0000
> header}
> body{
> part{ ID: 1, Content-type: text/plain
> @@ -155,7 +155,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-11) (inbox unread)
> Subject: Vs: thread-naming: Initial thread subject
> From: Notmuch Test Suite <[hidden email]>
> To: Notmuch Test Suite <[hidden email]>
> -Date: Thu, 11 Jan 2001 15:43:45 -0000
> +Date: Thu, 11 Jan 2001 15:43:45 +0000
> header}
> body{
> part{ ID: 1, Content-type: text/plain
> @@ -169,7 +169,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-12) (inbox unread)
> Subject: Sv: thread-naming: Initial thread subject
> From: Notmuch Test Suite <[hidden email]>
> To: Notmuch Test Suite <[hidden email]>
> -Date: Fri, 12 Jan 2001 15:43:45 -0000
> +Date: Fri, 12 Jan 2001 15:43:45 +0000
> header}
> body{
> part{ ID: 1, Content-type: text/plain
> --
> 1.7.7.3
>

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Austin Clements

Re: [PATCH 2/2] show: Simplify new text formatter code

Quoth Dmitry Kurochkin on Jan 31 at 3:37 am:

> On Thu, 26 Jan 2012 01:55:26 -0500, Austin Clements <[hidden email]> 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 <[hidden email]> (2000-01-01) (encrypted inbox)
> > Subject: test encrypted message 001
> > From: Notmuch Test Suite <[hidden email]>
> > To: [hidden email]
> > -Date: 01 Jan 2000 12:00:00 -0000
> > +Date: Sat, 01 Jan 2000 12:00:00 +0000
> > header}
> > body{
> > part{ 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 <[hidden email]> (2001-01-05) (unread)
> > Subject: thread-naming: Initial thread subject
> > From: Notmuch Test Suite <[hidden email]>
> > To: Notmuch Test Suite <[hidden email]>
> > -Date: Fri, 05 Jan 2001 15:43:56 -0000
> > +Date: Fri, 05 Jan 2001 15:43:56 +0000
> > header}
> > body{
> > part{ ID: 1, Content-type: text/plain
> > @@ -85,7 +85,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-06) (inbox unread)
> > Subject: thread-naming: Older changed subject
> > From: Notmuch Test Suite <[hidden email]>
> > To: Notmuch Test Suite <[hidden email]>
> > -Date: Sat, 06 Jan 2001 15:43:56 -0000
> > +Date: Sat, 06 Jan 2001 15:43:56 +0000
> > header}
> > body{
> > part{ ID: 1, Content-type: text/plain
> > @@ -99,7 +99,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-07) (inbox unread)
> > Subject: thread-naming: Newer changed subject
> > From: Notmuch Test Suite <[hidden email]>
> > To: Notmuch Test Suite <[hidden email]>
> > -Date: Sun, 07 Jan 2001 15:43:56 -0000
> > +Date: Sun, 07 Jan 2001 15:43:56 +0000
> > header}
> > body{
> > part{ ID: 1, Content-type: text/plain
> > @@ -113,7 +113,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-08) (unread)
> > Subject: thread-naming: Final thread subject
> > From: Notmuch Test Suite <[hidden email]>
> > To: Notmuch Test Suite <[hidden email]>
> > -Date: Mon, 08 Jan 2001 15:43:56 -0000
> > +Date: Mon, 08 Jan 2001 15:43:56 +0000
> > header}
> > body{
> > part{ ID: 1, Content-type: text/plain
> > @@ -127,7 +127,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-09) (inbox unread)
> > Subject: Re: thread-naming: Initial thread subject
> > From: Notmuch Test Suite <[hidden email]>
> > To: Notmuch Test Suite <[hidden email]>
> > -Date: Tue, 09 Jan 2001 15:43:45 -0000
> > +Date: Tue, 09 Jan 2001 15:43:45 +0000
> > header}
> > body{
> > part{ ID: 1, Content-type: text/plain
> > @@ -141,7 +141,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-10) (inbox unread)
> > Subject: Aw: thread-naming: Initial thread subject
> > From: Notmuch Test Suite <[hidden email]>
> > To: Notmuch Test Suite <[hidden email]>
> > -Date: Wed, 10 Jan 2001 15:43:45 -0000
> > +Date: Wed, 10 Jan 2001 15:43:45 +0000
> > header}
> > body{
> > part{ ID: 1, Content-type: text/plain
> > @@ -155,7 +155,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-11) (inbox unread)
> > Subject: Vs: thread-naming: Initial thread subject
> > From: Notmuch Test Suite <[hidden email]>
> > To: Notmuch Test Suite <[hidden email]>
> > -Date: Thu, 11 Jan 2001 15:43:45 -0000
> > +Date: Thu, 11 Jan 2001 15:43:45 +0000
> > header}
> > body{
> > part{ ID: 1, Content-type: text/plain
> > @@ -169,7 +169,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-12) (inbox unread)
> > Subject: Sv: thread-naming: Initial thread subject
> > From: Notmuch Test Suite <[hidden email]>
> > To: Notmuch Test Suite <[hidden email]>
> > -Date: Fri, 12 Jan 2001 15:43:45 -0000
> > +Date: Fri, 12 Jan 2001 15:43:45 +0000
> > header}
> > body{
> > part{ ID: 1, Content-type: text/plain
>

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Austin Clements

Re: [PATCH 1/2] show: Convert text format to the new self-recursive style

In reply to this post by Dmitry Kurochkin

Quoth Dmitry Kurochkin on Jan 31 at 3:26 am:

> On Thu, 26 Jan 2012 01:55:25 -0500, Austin Clements <[hidden email]> 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
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Austin Clements

[PATCH v2 0/2] Rewrite text show format

In reply to this post by Austin Clements

v2

* Remove unnecessary braces (id:[hidden email])

* Trivial rebase against master

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Austin Clements

[PATCH v2 1/2] show: Convert text format to the new self-recursive style

Austin Clements

[PATCH v2 2/2] show: Simplify new text formatter code

In reply to this post by Austin Clements

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 <[hidden email]> (2000-01-01) (encrypted inbox)
Subject: test encrypted message 001
From: Notmuch Test Suite <[hidden email]>
To: [hidden email]
-Date: 01 Jan 2000 12:00:00 -0000
+Date: Sat, 01 Jan 2000 12:00:00 +0000
header}
body{
part{ 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 <[hidden email]> (2001-01-05) (unread)
Subject: thread-naming: Initial thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Fri, 05 Jan 2001 15:43:56 -0000
+Date: Fri, 05 Jan 2001 15:43:56 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -85,7 +85,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-06) (inbox unread)
Subject: thread-naming: Older changed subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Sat, 06 Jan 2001 15:43:56 -0000
+Date: Sat, 06 Jan 2001 15:43:56 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -99,7 +99,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-07) (inbox unread)
Subject: thread-naming: Newer changed subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Sun, 07 Jan 2001 15:43:56 -0000
+Date: Sun, 07 Jan 2001 15:43:56 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -113,7 +113,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-08) (unread)
Subject: thread-naming: Final thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Mon, 08 Jan 2001 15:43:56 -0000
+Date: Mon, 08 Jan 2001 15:43:56 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -127,7 +127,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-09) (inbox unread)
Subject: Re: thread-naming: Initial thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Tue, 09 Jan 2001 15:43:45 -0000
+Date: Tue, 09 Jan 2001 15:43:45 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -141,7 +141,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-10) (inbox unread)
Subject: Aw: thread-naming: Initial thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Wed, 10 Jan 2001 15:43:45 -0000
+Date: Wed, 10 Jan 2001 15:43:45 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -155,7 +155,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-11) (inbox unread)
Subject: Vs: thread-naming: Initial thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Thu, 11 Jan 2001 15:43:45 -0000
+Date: Thu, 11 Jan 2001 15:43:45 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
@@ -169,7 +169,7 @@ Notmuch Test Suite <[hidden email]> (2001-01-12) (inbox unread)
Subject: Sv: thread-naming: Initial thread subject
From: Notmuch Test Suite <[hidden email]>
To: Notmuch Test Suite <[hidden email]>
-Date: Fri, 12 Jan 2001 15:43:45 -0000
+Date: Fri, 12 Jan 2001 15:43:45 +0000
header}
body{
part{ ID: 1, Content-type: text/plain
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Dmitry Kurochkin

Re: [PATCH v2 0/2] Rewrite text show format

In reply to this post by Austin Clements

On Sat, 4 Feb 2012 16:24:24 -0500, Austin Clements <[hidden email]> wrote:
> v2
>
> * Remove unnecessary braces (id:[hidden email])
>
> * 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
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

David Bremner-2

Re: [PATCH v2 0/2] Rewrite text show format

In reply to this post by Austin Clements

On Sat, 4 Feb 2012 16:24:24 -0500, Austin Clements <[hidden email]> wrote:
> v2
>
> * Remove unnecessary braces (id:[hidden email])
>
> * Trivial rebase against master

Pushed,

d
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

Adam Wolfe Gordon

Re: [PATCH 0/2] Rewrite text show format

In reply to this post by Austin Clements

Hi Austin,

On Wed, Jan 25, 2012 at 23:55, Austin Clements <[hidden email]> 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.
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

--
Adam Wolfe Gordon