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

2012-02-04 Thread Austin Clements
Quoth Dmitry Kurochkin on Jan 31 at  3:26 am:
> On Thu, 26 Jan 2012 01:55:25 -0500, Austin Clements  
> wrote:
> > This is all code movement and a smidgen of glue.  This moves the
> > existing text formatter code into one self-recursive function, but
> > doesn't change any of the logic.  The next patch will actually take
> > advantage of what the new structure has to offer.
> > 
> > Note that this patch retains format_headers_message_part_text because
> > it is also used by the raw format.
> > ---
> >  notmuch-show.c |  270 
> > +---
> >  1 files changed, 139 insertions(+), 131 deletions(-)
> > 
> > diff --git a/notmuch-show.c b/notmuch-show.c
> > index dec799c..6a890b2 100644
> > --- a/notmuch-show.c
> > +++ b/notmuch-show.c
> > @@ -21,40 +21,17 @@
> >  #include "notmuch-client.h"
> >  
> >  static void
> > -format_message_text (unused (const void *ctx),
> > -notmuch_message_t *message,
> > -int indent);
> > -static void
> > -format_headers_text (const void *ctx,
> > -notmuch_message_t *message);
> > -
> > -static void
> >  format_headers_message_part_text (GMimeMessage *message);
> >  
> >  static void
> > -format_part_start_text (GMimeObject *part,
> > -   int *part_count);
> > -
> > -static void
> > -format_part_content_text (GMimeObject *part);
> > -
> > -static void
> > -format_part_end_text (GMimeObject *part);
> > +format_part_text (const void *ctx, mime_node_t *node,
> > + int indent, const notmuch_show_params_t *params);
> >  
> >  static const notmuch_show_format_t format_text = {
> > -"", NULL,
> > -   "\fmessage{ ", format_message_text,
> > -   "\fheader{\n", format_headers_text, 
> > format_headers_message_part_text, "\fheader}\n",
> > -   "\fbody{\n",
> > -   format_part_start_text,
> > -   NULL,
> > -   NULL,
> > -   format_part_content_text,
> > -   format_part_end_text,
> > -   "",
> > -   "\fbody}\n",
> > -   "\fmessage}\n", "",
> > -""
> > +.message_set_start = "",
> > +.part = format_part_text,
> > +.message_set_sep = "",
> > +.message_set_end = ""
> 
> I guess I missed this during the first review.  I think we should
> support NULL values for message_set_* members (in a separate patch, I
> guess).  This would allow us to explicitly initialize only part member
> in the above code.

I wouldn't want to support this without supporting it for all of the
string members of notmuch_show_format_t, which turns out to be a
fairly big change.  At the end of the show rewrite, all of these other
string members will go away, so I'll add support for just these being
NULL at that point.

> Looks good otherwise.
> 
> Regards,
>   Dmitry


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

2012-02-04 Thread Austin Clements
Quoth Dmitry Kurochkin on Jan 31 at  3:26 am:
 On Thu, 26 Jan 2012 01:55:25 -0500, Austin Clements amdra...@mit.edu wrote:
  This is all code movement and a smidgen of glue.  This moves the
  existing text formatter code into one self-recursive function, but
  doesn't change any of the logic.  The next patch will actually take
  advantage of what the new structure has to offer.
  
  Note that this patch retains format_headers_message_part_text because
  it is also used by the raw format.
  ---
   notmuch-show.c |  270 
  +---
   1 files changed, 139 insertions(+), 131 deletions(-)
  
  diff --git a/notmuch-show.c b/notmuch-show.c
  index dec799c..6a890b2 100644
  --- a/notmuch-show.c
  +++ b/notmuch-show.c
  @@ -21,40 +21,17 @@
   #include notmuch-client.h
   
   static void
  -format_message_text (unused (const void *ctx),
  -notmuch_message_t *message,
  -int indent);
  -static void
  -format_headers_text (const void *ctx,
  -notmuch_message_t *message);
  -
  -static void
   format_headers_message_part_text (GMimeMessage *message);
   
   static void
  -format_part_start_text (GMimeObject *part,
  -   int *part_count);
  -
  -static void
  -format_part_content_text (GMimeObject *part);
  -
  -static void
  -format_part_end_text (GMimeObject *part);
  +format_part_text (const void *ctx, mime_node_t *node,
  + int indent, const notmuch_show_params_t *params);
   
   static const notmuch_show_format_t format_text = {
  -, NULL,
  -   \fmessage{ , format_message_text,
  -   \fheader{\n, format_headers_text, 
  format_headers_message_part_text, \fheader}\n,
  -   \fbody{\n,
  -   format_part_start_text,
  -   NULL,
  -   NULL,
  -   format_part_content_text,
  -   format_part_end_text,
  -   ,
  -   \fbody}\n,
  -   \fmessage}\n, ,
  -
  +.message_set_start = ,
  +.part = format_part_text,
  +.message_set_sep = ,
  +.message_set_end = 
 
 I guess I missed this during the first review.  I think we should
 support NULL values for message_set_* members (in a separate patch, I
 guess).  This would allow us to explicitly initialize only part member
 in the above code.

I wouldn't want to support this without supporting it for all of the
string members of notmuch_show_format_t, which turns out to be a
fairly big change.  At the end of the show rewrite, all of these other
string members will go away, so I'll add support for just these being
NULL at that point.

 Looks good otherwise.
 
 Regards,
   Dmitry
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


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

2012-01-31 Thread Dmitry Kurochkin
On Thu, 26 Jan 2012 01:55:25 -0500, Austin Clements  wrote:
> This is all code movement and a smidgen of glue.  This moves the
> existing text formatter code into one self-recursive function, but
> doesn't change any of the logic.  The next patch will actually take
> advantage of what the new structure has to offer.
> 
> Note that this patch retains format_headers_message_part_text because
> it is also used by the raw format.
> ---
>  notmuch-show.c |  270 
> +---
>  1 files changed, 139 insertions(+), 131 deletions(-)
> 
> diff --git a/notmuch-show.c b/notmuch-show.c
> index dec799c..6a890b2 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -21,40 +21,17 @@
>  #include "notmuch-client.h"
>  
>  static void
> -format_message_text (unused (const void *ctx),
> -  notmuch_message_t *message,
> -  int indent);
> -static void
> -format_headers_text (const void *ctx,
> -  notmuch_message_t *message);
> -
> -static void
>  format_headers_message_part_text (GMimeMessage *message);
>  
>  static void
> -format_part_start_text (GMimeObject *part,
> - int *part_count);
> -
> -static void
> -format_part_content_text (GMimeObject *part);
> -
> -static void
> -format_part_end_text (GMimeObject *part);
> +format_part_text (const void *ctx, mime_node_t *node,
> +   int indent, const notmuch_show_params_t *params);
>  
>  static const notmuch_show_format_t format_text = {
> -"", NULL,
> - "\fmessage{ ", format_message_text,
> - "\fheader{\n", format_headers_text, 
> format_headers_message_part_text, "\fheader}\n",
> - "\fbody{\n",
> - format_part_start_text,
> - NULL,
> - NULL,
> - format_part_content_text,
> - format_part_end_text,
> - "",
> - "\fbody}\n",
> - "\fmessage}\n", "",
> -""
> +.message_set_start = "",
> +.part = format_part_text,
> +.message_set_sep = "",
> +.message_set_end = ""

I guess I missed this during the first review.  I think we should
support NULL values for message_set_* members (in a separate patch, I
guess).  This would allow us to explicitly initialize only part member
in the above code.

Looks good otherwise.

Regards,
  Dmitry

>  };
>  
>  static void
> @@ -191,16 +168,6 @@ _get_one_line_summary (const void *ctx, 
> notmuch_message_t *message)
>  }
>  
>  static void
> -format_message_text (unused (const void *ctx), notmuch_message_t *message, 
> int indent)
> -{
> -printf ("id:%s depth:%d match:%d filename:%s\n",
> - notmuch_message_get_message_id (message),
> - indent,
> - notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
> - notmuch_message_get_filename (message));
> -}
> -
> -static void
>  format_message_json (const void *ctx, notmuch_message_t *message, unused 
> (int indent))
>  {
>  notmuch_tags_t *tags;
> @@ -338,26 +305,6 @@ format_message_mbox (const void *ctx,
>  fclose (file);
>  }
>  
> -
> -static void
> -format_headers_text (const void *ctx, notmuch_message_t *message)
> -{
> -const char *headers[] = {
> - "Subject", "From", "To", "Cc", "Bcc", "Date"
> -};
> -const char *name, *value;
> -unsigned int i;
> -
> -printf ("%s\n", _get_one_line_summary (ctx, message));
> -
> -for (i = 0; i < ARRAY_SIZE (headers); i++) {
> - name = headers[i];
> - value = notmuch_message_get_header (message, name);
> - if (value && strlen (value))
> - printf ("%s: %s\n", name, value);
> -}
> -}
> -
>  static void
>  format_headers_message_part_text (GMimeMessage *message)
>  {
> @@ -523,78 +470,6 @@ signer_status_to_string (GMimeSignerStatus x)
>  #endif
>  
>  static void
> -format_part_start_text (GMimeObject *part, int *part_count)
> -{
> -GMimeContentDisposition *disposition = 
> g_mime_object_get_content_disposition (part);
> -
> -if (disposition &&
> - strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
> -{
> - printf ("\fattachment{ ID: %d", *part_count);
> -
> -} else {
> -
> - printf ("\fpart{ ID: %d", *part_count);
> -}
> -}
> -
> -static void
> -format_part_content_text (GMimeObject *part)
> -{
> -const char *cid = g_mime_object_get_content_id (part);
> -GMimeContentType *content_type = g_mime_object_get_content_type 
> (GMIME_OBJECT (part));
> -
> -if (GMIME_IS_PART (part))
> -{
> - const char *filename = g_mime_part_get_filename (GMIME_PART (part));
> - if (filename)
> - printf (", Filename: %s", filename);
> -}
> -
> -if (cid)
> - printf (", Content-id: %s", cid);
> -
> -printf (", Content-type: %s\n", g_mime_content_type_to_string 
> (content_type));
> -
> -if (g_mime_content_type_is_type (content_type, "text", "*") &&
> - !g_mime_content_type_is_type (content_type, "text", "html"))
> -{
> - 

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

2012-01-26 Thread Austin Clements
This is all code movement and a smidgen of glue.  This moves the
existing text formatter code into one self-recursive function, but
doesn't change any of the logic.  The next patch will actually take
advantage of what the new structure has to offer.

Note that this patch retains format_headers_message_part_text because
it is also used by the raw format.
---
 notmuch-show.c |  270 +---
 1 files changed, 139 insertions(+), 131 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index dec799c..6a890b2 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -21,40 +21,17 @@
 #include "notmuch-client.h"

 static void
-format_message_text (unused (const void *ctx),
-notmuch_message_t *message,
-int indent);
-static void
-format_headers_text (const void *ctx,
-notmuch_message_t *message);
-
-static void
 format_headers_message_part_text (GMimeMessage *message);

 static void
-format_part_start_text (GMimeObject *part,
-   int *part_count);
-
-static void
-format_part_content_text (GMimeObject *part);
-
-static void
-format_part_end_text (GMimeObject *part);
+format_part_text (const void *ctx, mime_node_t *node,
+ int indent, const notmuch_show_params_t *params);

 static const notmuch_show_format_t format_text = {
-"", NULL,
-   "\fmessage{ ", format_message_text,
-   "\fheader{\n", format_headers_text, 
format_headers_message_part_text, "\fheader}\n",
-   "\fbody{\n",
-   format_part_start_text,
-   NULL,
-   NULL,
-   format_part_content_text,
-   format_part_end_text,
-   "",
-   "\fbody}\n",
-   "\fmessage}\n", "",
-""
+.message_set_start = "",
+.part = format_part_text,
+.message_set_sep = "",
+.message_set_end = ""
 };

 static void
@@ -191,16 +168,6 @@ _get_one_line_summary (const void *ctx, notmuch_message_t 
*message)
 }

 static void
-format_message_text (unused (const void *ctx), notmuch_message_t *message, int 
indent)
-{
-printf ("id:%s depth:%d match:%d filename:%s\n",
-   notmuch_message_get_message_id (message),
-   indent,
-   notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
-   notmuch_message_get_filename (message));
-}
-
-static void
 format_message_json (const void *ctx, notmuch_message_t *message, unused (int 
indent))
 {
 notmuch_tags_t *tags;
@@ -338,26 +305,6 @@ format_message_mbox (const void *ctx,
 fclose (file);
 }

-
-static void
-format_headers_text (const void *ctx, notmuch_message_t *message)
-{
-const char *headers[] = {
-   "Subject", "From", "To", "Cc", "Bcc", "Date"
-};
-const char *name, *value;
-unsigned int i;
-
-printf ("%s\n", _get_one_line_summary (ctx, message));
-
-for (i = 0; i < ARRAY_SIZE (headers); i++) {
-   name = headers[i];
-   value = notmuch_message_get_header (message, name);
-   if (value && strlen (value))
-   printf ("%s: %s\n", name, value);
-}
-}
-
 static void
 format_headers_message_part_text (GMimeMessage *message)
 {
@@ -523,78 +470,6 @@ signer_status_to_string (GMimeSignerStatus x)
 #endif

 static void
-format_part_start_text (GMimeObject *part, int *part_count)
-{
-GMimeContentDisposition *disposition = 
g_mime_object_get_content_disposition (part);
-
-if (disposition &&
-   strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
-{
-   printf ("\fattachment{ ID: %d", *part_count);
-
-} else {
-
-   printf ("\fpart{ ID: %d", *part_count);
-}
-}
-
-static void
-format_part_content_text (GMimeObject *part)
-{
-const char *cid = g_mime_object_get_content_id (part);
-GMimeContentType *content_type = g_mime_object_get_content_type 
(GMIME_OBJECT (part));
-
-if (GMIME_IS_PART (part))
-{
-   const char *filename = g_mime_part_get_filename (GMIME_PART (part));
-   if (filename)
-   printf (", Filename: %s", filename);
-}
-
-if (cid)
-   printf (", Content-id: %s", cid);
-
-printf (", Content-type: %s\n", g_mime_content_type_to_string 
(content_type));
-
-if (g_mime_content_type_is_type (content_type, "text", "*") &&
-   !g_mime_content_type_is_type (content_type, "text", "html"))
-{
-   GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
-   g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-   show_text_part_content (part, stream_stdout);
-   g_object_unref(stream_stdout);
-}
-else if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
-g_mime_content_type_is_type (content_type, "message", "rfc822"))
-{
-   /* Do nothing for multipart since its content will be printed
-* when recursing. */
-}
-else
-{
-   printf ("Non-text part: %s\n",
-   

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

2012-01-25 Thread Austin Clements
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