Re: [PATCH] doc: Allow rst2man.py as an alternative to rst2man
[notmuch-archives.git] / 39 / 1d834542b73872eca66a64c93633d2e9cf151d
1 Return-Path: <jani@nikula.org>\r
2 X-Original-To: notmuch@notmuchmail.org\r
3 Delivered-To: notmuch@notmuchmail.org\r
4 Received: from localhost (localhost [127.0.0.1])\r
5         by olra.theworths.org (Postfix) with ESMTP id A75F7431FAF\r
6         for <notmuch@notmuchmail.org>; Wed, 18 Jan 2012 14:25:56 -0800 (PST)\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
8 X-Spam-Flag: NO\r
9 X-Spam-Score: -0.7\r
10 X-Spam-Level: \r
11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
13 Received: from olra.theworths.org ([127.0.0.1])\r
14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
15         with ESMTP id PYASk-r6KkjV for <notmuch@notmuchmail.org>;\r
16         Wed, 18 Jan 2012 14:25:56 -0800 (PST)\r
17 Received: from mail-ey0-f181.google.com (mail-ey0-f181.google.com\r
18         [209.85.215.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
19         (No client certificate requested)\r
20         by olra.theworths.org (Postfix) with ESMTPS id D9E6C431FAE\r
21         for <notmuch@notmuchmail.org>; Wed, 18 Jan 2012 14:25:55 -0800 (PST)\r
22 Received: by eaal1 with SMTP id l1so483281eaa.26\r
23         for <notmuch@notmuchmail.org>; Wed, 18 Jan 2012 14:25:53 -0800 (PST)\r
24 Received: by 10.213.114.74 with SMTP id d10mr1081173ebq.128.1326925553085;\r
25         Wed, 18 Jan 2012 14:25:53 -0800 (PST)\r
26 Received: from localhost (dsl-hkibrasgw4-fe50f800-253.dhcp.inet.fi.\r
27         [84.248.80.253])\r
28         by mx.google.com with ESMTPS id s16sm105188335eef.2.2012.01.18.14.25.48\r
29         (version=SSLv3 cipher=OTHER); Wed, 18 Jan 2012 14:25:51 -0800 (PST)\r
30 From: Jani Nikula <jani@nikula.org>\r
31 To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org\r
32 Subject: Re: [PATCH 1/3] mime node: Record depth-first part numbers\r
33 In-Reply-To: <1326918507-28033-2-git-send-email-amdragon@mit.edu>\r
34 References: <1326918507-28033-1-git-send-email-amdragon@mit.edu>\r
35         <1326918507-28033-2-git-send-email-amdragon@mit.edu>\r
36 User-Agent: Notmuch/0.11+76~g1de742d (http://notmuchmail.org) Emacs/23.3.1\r
37         (i686-pc-linux-gnu)\r
38 Date: Thu, 19 Jan 2012 00:25:45 +0200\r
39 Message-ID: <87zkdkbqc6.fsf@nikula.org>\r
40 MIME-Version: 1.0\r
41 Content-Type: text/plain; charset=us-ascii\r
42 Cc: dkg@fifthhorseman.net\r
43 X-BeenThere: notmuch@notmuchmail.org\r
44 X-Mailman-Version: 2.1.13\r
45 Precedence: list\r
46 List-Id: "Use and development of the notmuch mail system."\r
47         <notmuch.notmuchmail.org>\r
48 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
49         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
50 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
51 List-Post: <mailto:notmuch@notmuchmail.org>\r
52 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
53 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
54         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
55 X-List-Received-Date: Wed, 18 Jan 2012 22:25:56 -0000\r
56 \r
57 On Wed, 18 Jan 2012 15:28:25 -0500, Austin Clements <amdragon@MIT.EDU> wrote:\r
58 > This makes the part numbers readily accessible to formatters.\r
59 > Hierarchical part numbering would be a more natural and efficient fit\r
60 > for MIME and may be the way to go in the future, but depth-first\r
61 > numbering maintains compatibility with what we currently do.\r
62 \r
63 Hi, please find a few things to consider below. If you disagree after\r
64 considering, it's quite all right, as they're largely style matters. :)\r
65 \r
66 BR,\r
67 Jani.\r
68 \r
69 \r
70 > ---\r
71 >  mime-node.c      |   33 ++++++++++++++++++++++++++++++++-\r
72 >  notmuch-client.h |   11 +++++++++++\r
73 >  2 files changed, 43 insertions(+), 1 deletions(-)\r
74\r
75 > diff --git a/mime-node.c b/mime-node.c\r
76 > index d26bb44..30b542f 100644\r
77 > --- a/mime-node.c\r
78 > +++ b/mime-node.c\r
79 > @@ -104,6 +104,10 @@ mime_node_open (const void *ctx, notmuch_message_t *message,\r
80 >      root->nchildren = 1;\r
81 >      root->ctx = mctx;\r
82 >  \r
83 > +    root->part_num = 0;\r
84 > +    root->next_child = 0;\r
85 > +    root->next_part_num = 1;\r
86 > +\r
87 >      *root_out = root;\r
88 >      return NOTMUCH_STATUS_SUCCESS;\r
89 >  \r
90 > @@ -133,6 +137,8 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part)\r
91 >       talloc_free (node);\r
92 >       return NULL;\r
93 >      }\r
94 > +    node->parent = parent;\r
95 > +    node->part_num = node->next_part_num = -1;\r
96 >  \r
97 >      /* Deal with the different types of parts */\r
98 >      if (GMIME_IS_PART (part)) {\r
99 > @@ -217,6 +223,7 @@ mime_node_t *\r
100 >  mime_node_child (const mime_node_t *parent, int child)\r
101 >  {\r
102 >      GMimeObject *sub;\r
103 > +    mime_node_t *node;\r
104 >  \r
105 >      if (!parent || child < 0 || child >= parent->nchildren)\r
106 >       return NULL;\r
107 > @@ -234,7 +241,31 @@ mime_node_child (const mime_node_t *parent, int child)\r
108 >       INTERNAL_ERROR ("Unexpected GMimeObject type: %s",\r
109 >                       g_type_name (G_OBJECT_TYPE (parent->part)));\r
110 >      }\r
111 > -    return _mime_node_create (parent, sub);\r
112 > +    node = _mime_node_create (parent, sub);\r
113 > +\r
114 > +    if (child == parent->next_child && parent->next_part_num != -1) {\r
115 > +     /* We're traversing in depth-first order.  Record the child's\r
116 > +      * depth-first numbering. */\r
117 > +     node->part_num = parent->next_part_num;\r
118 > +     node->next_part_num = node->part_num + 1;\r
119 > +\r
120 > +     /* Drop the const qualifier because these are internal fields\r
121 > +      * whose mutability doesn't affect the interface. */\r
122 \r
123 FWIW, I'm not a big fan of casting away const. Either it is const, or it\r
124 isn't. Not very many places would be affected if you dropped the const\r
125 qualifier from the related interface(s) altogether, and things would\r
126 look cleaner here. But I suppose this is a matter of taste.\r
127 \r
128 > +     ((mime_node_t*)parent)->next_child++;\r
129 > +     ((mime_node_t*)parent)->next_part_num = -1;\r
130 > +\r
131 > +     if (node->nchildren == 0) {\r
132 > +         /* We've reached a leaf, so find the parent that has more\r
133 > +          * children and set it up to number its next child. */\r
134 > +         const mime_node_t *it = node;\r
135 > +         while (it && it->next_child == it->nchildren)\r
136 > +             it = it->parent;\r
137 > +         if (it)\r
138 > +             ((mime_node_t*)it)->next_part_num = node->part_num + 1;\r
139 > +     }\r
140 > +    }\r
141 \r
142 Following the constness around made me wonder why the above if block\r
143 isn't within _mime_node_create(). It does have a feel of initialization\r
144 related to creation in it. (You'd have to pass more info to it though.)\r
145 \r
146 > +\r
147 > +    return node;\r
148 >  }\r
149 >  \r
150 >  static mime_node_t *\r
151 > diff --git a/notmuch-client.h b/notmuch-client.h\r
152 > index 62ede28..b3dcb6b 100644\r
153 > --- a/notmuch-client.h\r
154 > +++ b/notmuch-client.h\r
155 > @@ -281,6 +281,13 @@ typedef struct mime_node {\r
156 >      /* The number of children of this part. */\r
157 >      int nchildren;\r
158 >  \r
159 > +    /* The parent of this node or NULL if this is the root node. */\r
160 > +    const struct mime_node *parent;\r
161 > +\r
162 > +    /* The depth-first part number of this child if the MIME tree is\r
163 > +     * being traversed in depth-first order, or -1 otherwise. */\r
164 > +    int part_num;\r
165 > +\r
166 >      /* True if decryption of this part was attempted. */\r
167 >      notmuch_bool_t decrypt_attempted;\r
168 >      /* True if decryption of this part's child succeeded.  In this\r
169 > @@ -302,6 +309,10 @@ typedef struct mime_node {\r
170 >      /* Internal: For successfully decrypted multipart parts, the\r
171 >       * decrypted part to substitute for the second child. */\r
172 >      GMimeObject *decrypted_child;\r
173 > +\r
174 > +    /* Internal: The next child for depth-first traversal and the part\r
175 > +     * number to assign it (or -1 if unknown). */\r
176 > +    int next_child, next_part_num;\r
177 \r
178 A matter of taste again, but I wouldn't use "int foo, bar" in struct\r
179 declarations.\r
180 \r
181 >  } mime_node_t;\r
182 >  \r
183 >  /* Construct a new MIME node pointing to the root message part of\r
184 > -- \r
185 > 1.7.7.3\r
186\r