Re: Flat search and threaded views
[notmuch-archives.git] / 6c / 9a283562d53e19529d52f49edec490a5d87aea
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 D401D429E5B\r
6         for <notmuch@notmuchmail.org>; Wed,  1 Feb 2012 06:49:53 -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 5jdVGFK0o9U6 for <notmuch@notmuchmail.org>;\r
16         Wed,  1 Feb 2012 06:49:52 -0800 (PST)\r
17 Received: from mail-qy0-f181.google.com (mail-qy0-f181.google.com\r
18         [209.85.216.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 CB78E429E34\r
21         for <notmuch@notmuchmail.org>; Wed,  1 Feb 2012 06:49:51 -0800 (PST)\r
22 Received: by qcsd17 with SMTP id d17so814686qcs.26\r
23         for <notmuch@notmuchmail.org>; Wed, 01 Feb 2012 06:49:50 -0800 (PST)\r
24 Received: by 10.229.106.70 with SMTP id w6mr10238782qco.132.1328107789811;\r
25         Wed, 01 Feb 2012 06:49:49 -0800 (PST)\r
26 Received: from localhost (nikula.org. [92.243.24.172])\r
27         by mx.google.com with ESMTPS id dm7sm48023966qab.5.2012.02.01.06.49.46\r
28         (version=SSLv3 cipher=OTHER); Wed, 01 Feb 2012 06:49:48 -0800 (PST)\r
29 From: Jani Nikula <jani@nikula.org>\r
30 To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org\r
31 Subject: Re: [PATCH] added support for user-specified files & directories to\r
32         ignore\r
33 In-Reply-To: <20120131-new-ignore-1-git-send-email-too@iki.fi>\r
34 References: <1315949524-4948-1-git-send-email-tomi.ollila@iki.fi>\r
35         <20120131-new-ignore-1-git-send-email-too@iki.fi>\r
36 User-Agent: Notmuch/0.10.2+187~g43d4f26 (http://notmuchmail.org) Emacs/23.1.1\r
37         (i686-pc-linux-gnu)\r
38 Date: Wed, 01 Feb 2012 14:49:45 +0000\r
39 Message-ID: <87d39y622e.fsf@nikula.org>\r
40 MIME-Version: 1.0\r
41 Content-Type: text/plain; charset=us-ascii\r
42 Cc: Tomi Ollila <tomi.ollila@iki.fi>\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, 01 Feb 2012 14:49:54 -0000\r
56 \r
57 On Tue, 31 Jan 2012 18:28:04 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:\r
58 > A new configuration key 'new.ignore' is used to determine which\r
59 > files and directories user wants not to be scanned as new mails.\r
60 \r
61 Hi, please find a few drive-by "while my code is compiling" nitpick\r
62 review comments inline. ;) Mostly looks good.\r
63 \r
64 BR,\r
65 Jani.\r
66 \r
67 \r
68\r
69 > This work merges my previous attempts and Andreas Amann's work\r
70 > in id:"ylp7hi23mw8.fsf@tyndall.ie"\r
71\r
72 > See notes in id:"20120131-new-ignore-1-git-send-email-too@iki.fi"\r
73 > ---\r
74 > Notes\r
75\r
76 > 1) Currently there is comment for new.ignore in newly created configuration\r
77 > file but as the list is initially empty there will be not tag in place.\r
78\r
79 > 2) Whenever some already existing directory is added to the exclude list\r
80 > and the parent directory timestamp has not changed, notmuch new will not\r
81 > notice the directory has gone (as it still is there), user needs to 'touch'\r
82 > the parent directory before next 'notmuch new' no make notmuch notice.\r
83\r
84 > 2012-01-26: could notmuch track mtime of the configuration file and if\r
85 > that changes, ignore mail directory timestamps ?\r
86\r
87\r
88 > 3) in id:"1327572718-13411-2-git-send-email-tomi.ollila@iki.fi" dropped...\r
89\r
90 >  notmuch-client.h |    8 ++++++++\r
91 >  notmuch-config.c |   35 +++++++++++++++++++++++++++++++++--\r
92 >  notmuch-new.c    |   45 +++++++++++++++++++++++++++++++++------------\r
93 >  3 files changed, 74 insertions(+), 14 deletions(-)\r
94\r
95 > diff --git a/notmuch-client.h b/notmuch-client.h\r
96 > index e0eb594..c62ce78 100644\r
97 > --- a/notmuch-client.h\r
98 > +++ b/notmuch-client.h\r
99 > @@ -250,6 +250,14 @@ notmuch_config_set_new_tags (notmuch_config_t *config,\r
100 >                            const char *new_tags[],\r
101 >                            size_t length);\r
102 >  \r
103 > +const char **\r
104 > +notmuch_config_get_new_ignore (notmuch_config_t *config,\r
105 > +                            size_t *length);\r
106 > +void\r
107 > +notmuch_config_set_new_ignore (notmuch_config_t *config,\r
108 > +                            const char *new_ignore[],\r
109 > +                            size_t length);\r
110 > +\r
111 >  notmuch_bool_t\r
112 >  notmuch_config_get_maildir_synchronize_flags (notmuch_config_t *config);\r
113 >  \r
114 > diff --git a/notmuch-config.c b/notmuch-config.c\r
115 > index a124e34..f1cc5c2 100644\r
116 > --- a/notmuch-config.c\r
117 > +++ b/notmuch-config.c\r
118 > @@ -44,7 +44,10 @@ static const char new_config_comment[] =\r
119 >      " The following options are supported here:\n"\r
120 >      "\n"\r
121 >      "\ttags  A list (separated by ';') of the tags that will be\n"\r
122 > -    "\t      added to all messages incorporated by \"notmuch new\".\n";\r
123 > +    "\t      added to all messages incorporated by \"notmuch new\".\n"\r
124 > +    "\n"\r
125 > +    "\tignore        A list (separated by ';') of files and directories that"\r
126 > +    "\t      will not be searched for messages by \"notmuch new\".\n";\r
127 >  \r
128 >  static const char user_config_comment[] =\r
129 >      " User configuration\n"\r
130 > @@ -105,6 +108,8 @@ struct _notmuch_config {\r
131 >      size_t user_other_email_length;\r
132 >      const char **new_tags;\r
133 >      size_t new_tags_length;\r
134 > +    const char **new_ignore;\r
135 > +    size_t new_ignore_length;\r
136 >      notmuch_bool_t maildir_synchronize_flags;\r
137 >      const char **search_exclude_tags;\r
138 >      size_t search_exclude_tags_length;\r
139 > @@ -264,6 +269,8 @@ notmuch_config_open (void *ctx,\r
140 >      config->user_other_email_length = 0;\r
141 >      config->new_tags = NULL;\r
142 >      config->new_tags_length = 0;\r
143 > +    config->new_ignore = NULL;\r
144 > +    config->new_ignore_length = 0;\r
145 >      config->maildir_synchronize_flags = TRUE;\r
146 >      config->search_exclude_tags = NULL;\r
147 >      config->search_exclude_tags_length = 0;\r
148 > @@ -360,7 +367,11 @@ notmuch_config_open (void *ctx,\r
149 >          const char *tags[] = { "unread", "inbox" };\r
150 >       notmuch_config_set_new_tags (config, tags, 2);\r
151 >      }\r
152 > -\r
153 > +#if 0 /* No point setting empty list -- it's not written */\r
154 > +    if (notmuch_config_get_new_ignore (config, &tmp) == NULL) {\r
155 > +     notmuch_config_set_new_ignore (config, NULL, 0);\r
156 > +    }\r
157 > +#endif\r
158 \r
159 I'd prefer not having any #if 0 code. How about just having the code\r
160 there, even if it ends up being a NOP? You never know when the config\r
161 plumbing is going to change (it could use some love), e.g. it might\r
162 write the default value in comments in the future.\r
163 \r
164 >      if (notmuch_config_get_search_exclude_tags (config, &tmp) == NULL) {\r
165 >       if (is_new) {\r
166 >           const char *tags[] = { "deleted", "spam" };\r
167 > @@ -609,6 +620,15 @@ notmuch_config_get_new_tags (notmuch_config_t *config,   size_t *length)\r
168 >                            &(config->new_tags_length), length);\r
169 >  }\r
170 >  \r
171 > +const char **\r
172 > +notmuch_config_get_new_ignore (notmuch_config_t *config,   size_t *length)\r
173 \r
174 Is there extra space in there?\r
175 \r
176 > +{\r
177 > +    return _config_get_list (config, "new", "ignore",\r
178 > +                          &(config->new_ignore),\r
179 > +                          &(config->new_ignore_length), length);\r
180 > +}\r
181 > +\r
182 > +\r
183 >  void\r
184 >  notmuch_config_set_user_other_email (notmuch_config_t *config,\r
185 >                                    const char *list[],\r
186 > @@ -627,6 +647,17 @@ notmuch_config_set_new_tags (notmuch_config_t *config,\r
187 >                    &(config->new_tags));\r
188 >  }\r
189 >  \r
190 > +#if 0 /* UNNEEDED SO FAR */\r
191 > +void\r
192 > +notmuch_config_set_new_ignore (notmuch_config_t *config,\r
193 > +                            const char *list[],\r
194 > +                            size_t length)\r
195 > +{\r
196 > +    _config_set_list (config, "new", "ignore", list, length,\r
197 > +                  &(config->new_ignore));\r
198 > +}\r
199 > +#endif\r
200 \r
201 Same here, just remove the #if 0.\r
202 \r
203 > +\r
204 >  const char **\r
205 >  notmuch_config_get_search_exclude_tags (notmuch_config_t *config, size_t *length)\r
206 >  {\r
207 > diff --git a/notmuch-new.c b/notmuch-new.c\r
208 > index a569a54..36d5c5d 100644\r
209 > --- a/notmuch-new.c\r
210 > +++ b/notmuch-new.c\r
211 > @@ -39,6 +39,8 @@ typedef struct {\r
212 >      int verbose;\r
213 >      const char **new_tags;\r
214 >      size_t new_tags_length;\r
215 > +    const char **new_ignore;\r
216 > +    size_t new_ignore_length;\r
217 >  \r
218 >      int total_files;\r
219 >      int processed_files;\r
220 > @@ -181,6 +183,20 @@ _entries_resemble_maildir (struct dirent **entries, int count)\r
221 >      return 0;\r
222 >  }\r
223 >  \r
224 > +/* Check if user asked to ignore these files/directories */\r
225 > +\r
226 > +static int\r
227 \r
228 notmuch_bool_t?\r
229 \r
230 > +_entry_in_ignore_list (const char *entry, add_files_state_t *state)\r
231 > +{\r
232 > +    size_t i, ignore_length = state->new_ignore_length;\r
233 \r
234 ignore_length is an unnecessary temp variable.\r
235 \r
236 > +\r
237 > +    for (i = 0; i < ignore_length; i++)\r
238 > +     if (strcmp (entry, state->new_ignore[i]) == 0)\r
239 > +         return 1;\r
240 > +\r
241 > +    return 0;\r
242 > +}\r
243 > +\r
244 >  /* Examine 'path' recursively as follows:\r
245 >   *\r
246 >   *   o Ask the filesystem for the mtime of 'path' (fs_mtime)\r
247 > @@ -320,15 +336,15 @@ add_files_recursive (notmuch_database_t *notmuch,\r
248 >       }\r
249 >  \r
250 >       /* Ignore special directories to avoid infinite recursion.\r
251 > -      * Also ignore the .notmuch directory and any "tmp" directory\r
252 > -      * that appears within a maildir.\r
253 > +      * Also ignore the .notmuch directory, any "tmp" directory\r
254 > +      * that appears within a maildir and files/directories\r
255 > +      * user have configured to be ignored.\r
256 \r
257 s/user have/the user has/ ?\r
258 \r
259 >        */\r
260 > -     /* XXX: Eventually we'll want more sophistication to let the\r
261 > -      * user specify files to be ignored. */\r
262 >       if (strcmp (entry->d_name, ".") == 0 ||\r
263 >           strcmp (entry->d_name, "..") == 0 ||\r
264 >           (is_maildir && strcmp (entry->d_name, "tmp") == 0) ||\r
265 > -         strcmp (entry->d_name, ".notmuch") ==0)\r
266 > +         strcmp (entry->d_name, ".notmuch") == 0 ||\r
267 > +         _entry_in_ignore_list (entry->d_name, state))\r
268 >       {\r
269 >           continue;\r
270 >       }\r
271 > @@ -369,6 +385,10 @@ add_files_recursive (notmuch_database_t *notmuch,\r
272 >  \r
273 >          entry = fs_entries[i];\r
274 >  \r
275 > +     /* Ignore files & directories user has configured to be ignored */\r
276 > +     if (_entry_in_ignore_list (entry->d_name, state))\r
277 > +         continue;\r
278 > +\r
279 >       /* Check if we've walked past any names in db_files or\r
280 >        * db_subdirs. If so, these have been deleted. */\r
281 >       while (notmuch_filenames_valid (db_files) &&\r
282 > @@ -648,7 +668,7 @@ add_files (notmuch_database_t *notmuch,\r
283 >   * initialized to zero by the top-level caller before calling\r
284 >   * count_files). */\r
285 >  static void\r
286 > -count_files (const char *path, int *count)\r
287 > +count_files (const char *path, int *count, add_files_state_t *state)\r
288 >  {\r
289 >      struct dirent *entry = NULL;\r
290 >      char *next;\r
291 > @@ -670,13 +690,13 @@ count_files (const char *path, int *count)\r
292 >          entry = fs_entries[i++];\r
293 >  \r
294 >       /* Ignore special directories to avoid infinite recursion.\r
295 > -      * Also ignore the .notmuch directory.\r
296 > +      * Also ignore the .notmuch directory and files/directories\r
297 > +      * user have configured to be ignored.\r
298 \r
299 Same as above.\r
300 \r
301 >        */\r
302 > -     /* XXX: Eventually we'll want more sophistication to let the\r
303 > -      * user specify files to be ignored. */\r
304 \r
305 Oh, you think this is enough sophistication! ;)\r
306 \r
307 >       if (strcmp (entry->d_name, ".") == 0 ||\r
308 >           strcmp (entry->d_name, "..") == 0 ||\r
309 > -         strcmp (entry->d_name, ".notmuch") == 0)\r
310 > +         strcmp (entry->d_name, ".notmuch") == 0 ||\r
311 > +         _entry_in_ignore_list (entry->d_name, state))\r
312 >       {\r
313 >           continue;\r
314 >       }\r
315 > @@ -697,7 +717,7 @@ count_files (const char *path, int *count)\r
316 >               fflush (stdout);\r
317 >           }\r
318 >       } else if (S_ISDIR (st.st_mode)) {\r
319 > -         count_files (next, count);\r
320 > +         count_files (next, count, state);\r
321 >       }\r
322 >  \r
323 >       free (next);\r
324 > @@ -837,6 +857,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])\r
325 >       return 1;\r
326 >  \r
327 >      add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length);\r
328 > +    add_files_state.new_ignore = notmuch_config_get_new_ignore (config, &add_files_state.new_ignore_length);\r
329 >      add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);\r
330 >      db_path = notmuch_config_get_database_path (config);\r
331 >  \r
332 > @@ -852,7 +873,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])\r
333 >       int count;\r
334 >  \r
335 >       count = 0;\r
336 > -     count_files (db_path, &count);\r
337 > +     count_files (db_path, &count, &add_files_state);\r
338 >       if (interrupted)\r
339 >           return 1;\r
340 >  \r
341 > -- \r
342 > 1.7.6.5\r
343\r
344 > _______________________________________________\r
345 > notmuch mailing list\r
346 > notmuch@notmuchmail.org\r
347 > http://notmuchmail.org/mailman/listinfo/notmuch\r