[PATCH v2 3/3] show: Introduce mime_node formatter callback
On Sun, 22 Jan 2012 21:31:13 -0500, Austin Clements wrote:
> This callback is the gateway to the new mime_node_t-based formatters.
> This maintains backwards compatibility so the formatters can be
> transitioned one at a time. Once all formatters are converted, the
> formatter structure can be reduced to only message_set_{start,sep,end}
> and part, most of show_message can be deleted, and all of
> show-message.c can be deleted.
Few comments below.
> ---
> notmuch-client.h |6 ++
> notmuch-reply.c |2 +-
> notmuch-show.c | 23 +++
> 3 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index abfe5d3..59606b4 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -62,8 +62,14 @@
> #define STRINGIFY(s) STRINGIFY_(s)
> #define STRINGIFY_(s) #s
>
> +struct mime_node;
> +struct notmuch_show_params;
> +
> typedef struct notmuch_show_format {
> const char *message_set_start;
> +void (*part) (const void *ctx,
> + struct mime_node *node, int indent,
> + struct notmuch_show_params *params);
Can we make the params parameter const?
> const char *message_start;
> void (*message) (const void *ctx,
>notmuch_message_t *message,
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index bf67960..f55b1d2 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -31,7 +31,7 @@ static void
> reply_part_content (GMimeObject *part);
>
> static const notmuch_show_format_t format_reply = {
> -"",
> +"", NULL,
> "", NULL,
> "", NULL, reply_headers_message_part, ">\n",
> "",
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 682aa71..8185b02 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -42,7 +42,7 @@ static void
> format_part_end_text (GMimeObject *part);
>
> 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",
> @@ -89,7 +89,7 @@ static void
> format_part_end_json (GMimeObject *part);
>
> static const notmuch_show_format_t format_json = {
> -"[",
> +"[", NULL,
> "{", format_message_json,
> "\"headers\": {", format_headers_json,
> format_headers_message_part_json, "}",
> ", \"body\": [",
> @@ -110,7 +110,7 @@ format_message_mbox (const void *ctx,
>unused (int indent));
>
> static const notmuch_show_format_t format_mbox = {
> -"",
> +"", NULL,
> "", format_message_mbox,
> "", NULL, NULL, "",
> "",
> @@ -129,7 +129,7 @@ static void
> format_part_content_raw (GMimeObject *part);
>
> static const notmuch_show_format_t format_raw = {
> -"",
> +"", NULL,
> "", NULL,
> "", NULL, format_headers_message_part_text, "\n",
> "",
> @@ -850,6 +850,21 @@ show_message (void *ctx,
> int indent,
> notmuch_show_params_t *params)
> {
> +if (format->part) {
> + void *local = talloc_new (ctx);
> + mime_node_t *root, *part;
> +
> + if (mime_node_open (local, message, params->cryptoctx, params->decrypt,
> + ) != NOTMUCH_STATUS_SUCCESS)
> + goto DONE;
> + part = mime_node_seek_dfs (root, params->part < 0 ? 0 : params->part);
This should be done as a separate patch, but it looks like zero and
negative params->part is handled in the same way so we can change it to
always be non-negative.
> + if (part)
> + format->part (local, part, indent, params);
> + DONE:
> + talloc_free (local);
> + return;
Please move part assignment inside the if and remove the goto:
if (mime_node_open() == success && (part = seek()))
format->part();
Regards,
Dmitry
> +}
> +
> if (params->part <= 0) {
> fputs (format->message_start, stdout);
> if (format->message)
> --
> 1.7.7.3
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 3/3] show: Introduce mime_node formatter callback
Thanks for the review. New version coming shortly (v4, since the list
ate v3 while everyone was still reading v2).
Quoth Dmitry Kurochkin on Jan 24 at 2:37 am:
On Sun, 22 Jan 2012 21:31:13 -0500, Austin Clements amdra...@mit.edu wrote:
This callback is the gateway to the new mime_node_t-based formatters.
This maintains backwards compatibility so the formatters can be
transitioned one at a time. Once all formatters are converted, the
formatter structure can be reduced to only message_set_{start,sep,end}
and part, most of show_message can be deleted, and all of
show-message.c can be deleted.
Few comments below.
---
notmuch-client.h |6 ++
notmuch-reply.c |2 +-
notmuch-show.c | 23 +++
3 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/notmuch-client.h b/notmuch-client.h
index abfe5d3..59606b4 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -62,8 +62,14 @@
#define STRINGIFY(s) STRINGIFY_(s)
#define STRINGIFY_(s) #s
+struct mime_node;
+struct notmuch_show_params;
+
typedef struct notmuch_show_format {
const char *message_set_start;
+void (*part) (const void *ctx,
+ struct mime_node *node, int indent,
+ struct notmuch_show_params *params);
Can we make the params parameter const?
Apparently we can.
const char *message_start;
void (*message) (const void *ctx,
notmuch_message_t *message,
diff --git a/notmuch-reply.c b/notmuch-reply.c
index bf67960..f55b1d2 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -31,7 +31,7 @@ static void
reply_part_content (GMimeObject *part);
static const notmuch_show_format_t format_reply = {
-,
+, NULL,
, NULL,
, NULL, reply_headers_message_part, \n,
,
diff --git a/notmuch-show.c b/notmuch-show.c
index 682aa71..8185b02 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -42,7 +42,7 @@ static void
format_part_end_text (GMimeObject *part);
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,
@@ -89,7 +89,7 @@ static void
format_part_end_json (GMimeObject *part);
static const notmuch_show_format_t format_json = {
-[,
+[, NULL,
{, format_message_json,
\headers\: {, format_headers_json,
format_headers_message_part_json, },
, \body\: [,
@@ -110,7 +110,7 @@ format_message_mbox (const void *ctx,
unused (int indent));
static const notmuch_show_format_t format_mbox = {
-,
+, NULL,
, format_message_mbox,
, NULL, NULL, ,
,
@@ -129,7 +129,7 @@ static void
format_part_content_raw (GMimeObject *part);
static const notmuch_show_format_t format_raw = {
-,
+, NULL,
, NULL,
, NULL, format_headers_message_part_text, \n,
,
@@ -850,6 +850,21 @@ show_message (void *ctx,
int indent,
notmuch_show_params_t *params)
{
+if (format-part) {
+ void *local = talloc_new (ctx);
+ mime_node_t *root, *part;
+
+ if (mime_node_open (local, message, params-cryptoctx, params-decrypt,
+ root) != NOTMUCH_STATUS_SUCCESS)
+ goto DONE;
+ part = mime_node_seek_dfs (root, params-part 0 ? 0 : params-part);
This should be done as a separate patch, but it looks like zero and
negative params-part is handled in the same way so we can change it to
always be non-negative.
That's true here, but there are other places where the difference does
matter. (I would certainly *prefer* this not to be asymmetric, but
that's a bigger issue involving show's inconsistent interface.)
+ if (part)
+ format-part (local, part, indent, params);
+ DONE:
+ talloc_free (local);
+ return;
Please move part assignment inside the if and remove the goto:
if (mime_node_open() == success (part = seek()))
format-part();
Done.
Regards,
Dmitry
+}
+
if (params-part = 0) {
fputs (format-message_start, stdout);
if (format-message)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 3/3] show: Introduce mime_node formatter callback
On Sun, 22 Jan 2012 21:31:13 -0500, Austin Clements wrote:
> This callback is the gateway to the new mime_node_t-based formatters.
> This maintains backwards compatibility so the formatters can be
> transitioned one at a time. Once all formatters are converted, the
> formatter structure can be reduced to only message_set_{start,sep,end}
> and part, most of show_message can be deleted, and all of
> show-message.c can be deleted.
> ---
LGTM.
Tomi
[PATCH v2 3/3] show: Introduce mime_node formatter callback
Thanks for the review. New version coming shortly (v4, since the list
ate v3 while everyone was still reading v2).
Quoth Dmitry Kurochkin on Jan 24 at 2:37 am:
> On Sun, 22 Jan 2012 21:31:13 -0500, Austin Clements
> wrote:
> > This callback is the gateway to the new mime_node_t-based formatters.
> > This maintains backwards compatibility so the formatters can be
> > transitioned one at a time. Once all formatters are converted, the
> > formatter structure can be reduced to only message_set_{start,sep,end}
> > and part, most of show_message can be deleted, and all of
> > show-message.c can be deleted.
>
> Few comments below.
>
> > ---
> > notmuch-client.h |6 ++
> > notmuch-reply.c |2 +-
> > notmuch-show.c | 23 +++
> > 3 files changed, 26 insertions(+), 5 deletions(-)
> >
> > diff --git a/notmuch-client.h b/notmuch-client.h
> > index abfe5d3..59606b4 100644
> > --- a/notmuch-client.h
> > +++ b/notmuch-client.h
> > @@ -62,8 +62,14 @@
> > #define STRINGIFY(s) STRINGIFY_(s)
> > #define STRINGIFY_(s) #s
> >
> > +struct mime_node;
> > +struct notmuch_show_params;
> > +
> > typedef struct notmuch_show_format {
> > const char *message_set_start;
> > +void (*part) (const void *ctx,
> > + struct mime_node *node, int indent,
> > + struct notmuch_show_params *params);
>
> Can we make the params parameter const?
Apparently we can.
> > const char *message_start;
> > void (*message) (const void *ctx,
> > notmuch_message_t *message,
> > diff --git a/notmuch-reply.c b/notmuch-reply.c
> > index bf67960..f55b1d2 100644
> > --- a/notmuch-reply.c
> > +++ b/notmuch-reply.c
> > @@ -31,7 +31,7 @@ static void
> > reply_part_content (GMimeObject *part);
> >
> > static const notmuch_show_format_t format_reply = {
> > -"",
> > +"", NULL,
> > "", NULL,
> > "", NULL, reply_headers_message_part, ">\n",
> > "",
> > diff --git a/notmuch-show.c b/notmuch-show.c
> > index 682aa71..8185b02 100644
> > --- a/notmuch-show.c
> > +++ b/notmuch-show.c
> > @@ -42,7 +42,7 @@ static void
> > format_part_end_text (GMimeObject *part);
> >
> > 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",
> > @@ -89,7 +89,7 @@ static void
> > format_part_end_json (GMimeObject *part);
> >
> > static const notmuch_show_format_t format_json = {
> > -"[",
> > +"[", NULL,
> > "{", format_message_json,
> > "\"headers\": {", format_headers_json,
> > format_headers_message_part_json, "}",
> > ", \"body\": [",
> > @@ -110,7 +110,7 @@ format_message_mbox (const void *ctx,
> > unused (int indent));
> >
> > static const notmuch_show_format_t format_mbox = {
> > -"",
> > +"", NULL,
> > "", format_message_mbox,
> > "", NULL, NULL, "",
> > "",
> > @@ -129,7 +129,7 @@ static void
> > format_part_content_raw (GMimeObject *part);
> >
> > static const notmuch_show_format_t format_raw = {
> > -"",
> > +"", NULL,
> > "", NULL,
> > "", NULL, format_headers_message_part_text, "\n",
> > "",
> > @@ -850,6 +850,21 @@ show_message (void *ctx,
> > int indent,
> > notmuch_show_params_t *params)
> > {
> > +if (format->part) {
> > + void *local = talloc_new (ctx);
> > + mime_node_t *root, *part;
> > +
> > + if (mime_node_open (local, message, params->cryptoctx, params->decrypt,
> > + ) != NOTMUCH_STATUS_SUCCESS)
> > + goto DONE;
> > + part = mime_node_seek_dfs (root, params->part < 0 ? 0 : params->part);
>
> This should be done as a separate patch, but it looks like zero and
> negative params->part is handled in the same way so we can change it to
> always be non-negative.
That's true here, but there are other places where the difference does
matter. (I would certainly *prefer* this not to be asymmetric, but
that's a bigger issue involving show's inconsistent interface.)
> > + if (part)
> > + format->part (local, part, indent, params);
> > + DONE:
> > + talloc_free (local);
> > + return;
>
> Please move part assignment inside the if and remove the goto:
>
> if (mime_node_open() == success && (part = seek()))
> format->part();
Done.
> Regards,
> Dmitry
>
> > +}
> > +
> > if (params->part <= 0) {
> > fputs (format->message_start, stdout);
> > if (format->message)
>
[PATCH v2 3/3] show: Introduce mime_node formatter callback
This callback is the gateway to the new mime_node_t-based formatters.
This maintains backwards compatibility so the formatters can be
transitioned one at a time. Once all formatters are converted, the
formatter structure can be reduced to only message_set_{start,sep,end}
and part, most of show_message can be deleted, and all of
show-message.c can be deleted.
---
notmuch-client.h |6 ++
notmuch-reply.c |2 +-
notmuch-show.c | 23 +++
3 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/notmuch-client.h b/notmuch-client.h
index abfe5d3..59606b4 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -62,8 +62,14 @@
#define STRINGIFY(s) STRINGIFY_(s)
#define STRINGIFY_(s) #s
+struct mime_node;
+struct notmuch_show_params;
+
typedef struct notmuch_show_format {
const char *message_set_start;
+void (*part) (const void *ctx,
+ struct mime_node *node, int indent,
+ struct notmuch_show_params *params);
const char *message_start;
void (*message) (const void *ctx,
notmuch_message_t *message,
diff --git a/notmuch-reply.c b/notmuch-reply.c
index bf67960..f55b1d2 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -31,7 +31,7 @@ static void
reply_part_content (GMimeObject *part);
static const notmuch_show_format_t format_reply = {
-"",
+"", NULL,
"", NULL,
"", NULL, reply_headers_message_part, ">\n",
"",
diff --git a/notmuch-show.c b/notmuch-show.c
index 682aa71..8185b02 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -42,7 +42,7 @@ static void
format_part_end_text (GMimeObject *part);
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",
@@ -89,7 +89,7 @@ static void
format_part_end_json (GMimeObject *part);
static const notmuch_show_format_t format_json = {
-"[",
+"[", NULL,
"{", format_message_json,
"\"headers\": {", format_headers_json,
format_headers_message_part_json, "}",
", \"body\": [",
@@ -110,7 +110,7 @@ format_message_mbox (const void *ctx,
unused (int indent));
static const notmuch_show_format_t format_mbox = {
-"",
+"", NULL,
"", format_message_mbox,
"", NULL, NULL, "",
"",
@@ -129,7 +129,7 @@ static void
format_part_content_raw (GMimeObject *part);
static const notmuch_show_format_t format_raw = {
-"",
+"", NULL,
"", NULL,
"", NULL, format_headers_message_part_text, "\n",
"",
@@ -850,6 +850,21 @@ show_message (void *ctx,
int indent,
notmuch_show_params_t *params)
{
+if (format->part) {
+ void *local = talloc_new (ctx);
+ mime_node_t *root, *part;
+
+ if (mime_node_open (local, message, params->cryptoctx, params->decrypt,
+ ) != NOTMUCH_STATUS_SUCCESS)
+ goto DONE;
+ part = mime_node_seek_dfs (root, params->part < 0 ? 0 : params->part);
+ if (part)
+ format->part (local, part, indent, params);
+ DONE:
+ talloc_free (local);
+ return;
+}
+
if (params->part <= 0) {
fputs (format->message_start, stdout);
if (format->message)
--
1.7.7.3
[PATCH v2 3/3] show: Introduce mime_node formatter callback
This callback is the gateway to the new mime_node_t-based formatters.
This maintains backwards compatibility so the formatters can be
transitioned one at a time. Once all formatters are converted, the
formatter structure can be reduced to only message_set_{start,sep,end}
and part, most of show_message can be deleted, and all of
show-message.c can be deleted.
---
notmuch-client.h |6 ++
notmuch-reply.c |2 +-
notmuch-show.c | 23 +++
3 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/notmuch-client.h b/notmuch-client.h
index abfe5d3..59606b4 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -62,8 +62,14 @@
#define STRINGIFY(s) STRINGIFY_(s)
#define STRINGIFY_(s) #s
+struct mime_node;
+struct notmuch_show_params;
+
typedef struct notmuch_show_format {
const char *message_set_start;
+void (*part) (const void *ctx,
+ struct mime_node *node, int indent,
+ struct notmuch_show_params *params);
const char *message_start;
void (*message) (const void *ctx,
notmuch_message_t *message,
diff --git a/notmuch-reply.c b/notmuch-reply.c
index bf67960..f55b1d2 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -31,7 +31,7 @@ static void
reply_part_content (GMimeObject *part);
static const notmuch_show_format_t format_reply = {
-,
+, NULL,
, NULL,
, NULL, reply_headers_message_part, \n,
,
diff --git a/notmuch-show.c b/notmuch-show.c
index 682aa71..8185b02 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -42,7 +42,7 @@ static void
format_part_end_text (GMimeObject *part);
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,
@@ -89,7 +89,7 @@ static void
format_part_end_json (GMimeObject *part);
static const notmuch_show_format_t format_json = {
-[,
+[, NULL,
{, format_message_json,
\headers\: {, format_headers_json,
format_headers_message_part_json, },
, \body\: [,
@@ -110,7 +110,7 @@ format_message_mbox (const void *ctx,
unused (int indent));
static const notmuch_show_format_t format_mbox = {
-,
+, NULL,
, format_message_mbox,
, NULL, NULL, ,
,
@@ -129,7 +129,7 @@ static void
format_part_content_raw (GMimeObject *part);
static const notmuch_show_format_t format_raw = {
-,
+, NULL,
, NULL,
, NULL, format_headers_message_part_text, \n,
,
@@ -850,6 +850,21 @@ show_message (void *ctx,
int indent,
notmuch_show_params_t *params)
{
+if (format-part) {
+ void *local = talloc_new (ctx);
+ mime_node_t *root, *part;
+
+ if (mime_node_open (local, message, params-cryptoctx, params-decrypt,
+ root) != NOTMUCH_STATUS_SUCCESS)
+ goto DONE;
+ part = mime_node_seek_dfs (root, params-part 0 ? 0 : params-part);
+ if (part)
+ format-part (local, part, indent, params);
+ DONE:
+ talloc_free (local);
+ return;
+}
+
if (params-part = 0) {
fputs (format-message_start, stdout);
if (format-message)
--
1.7.7.3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
