--- /dev/null
+Return-Path: <jani@nikula.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id 3D06E431FC4\r
+ for <notmuch@notmuchmail.org>; Thu, 2 Feb 2012 09:16:11 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: 0\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
+ autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id JQTmLysoQeK8 for <notmuch@notmuchmail.org>;\r
+ Thu, 2 Feb 2012 09:16:09 -0800 (PST)\r
+Received: from mail-lpp01m020-f181.google.com (mail-lpp01m020-f181.google.com\r
+ [209.85.217.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+ (No client certificate requested)\r
+ by olra.theworths.org (Postfix) with ESMTPS id 66590431FAE\r
+ for <notmuch@notmuchmail.org>; Thu, 2 Feb 2012 09:16:09 -0800 (PST)\r
+Received: by lbbgn5 with SMTP id gn5so587603lbb.26\r
+ for <notmuch@notmuchmail.org>; Thu, 02 Feb 2012 09:16:07 -0800 (PST)\r
+Received: by 10.112.25.8 with SMTP id y8mr963172lbf.15.1328202967818;\r
+ Thu, 02 Feb 2012 09:16:07 -0800 (PST)\r
+Received: from localhost (dsl-hkibrasgw4-fe50f800-253.dhcp.inet.fi.\r
+ [84.248.80.253])\r
+ by mx.google.com with ESMTPS id a8sm2466688lba.15.2012.02.02.09.16.04\r
+ (version=SSLv3 cipher=OTHER); Thu, 02 Feb 2012 09:16:06 -0800 (PST)\r
+From: Jani Nikula <jani@nikula.org>\r
+To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH] add support for user-specified files & directories to\r
+ ignore\r
+In-Reply-To: <20120202-new-ignore-1-git-send-email-too@iki.fi>\r
+References: <id:"1315949524-4948-1-git-send-email-tomi.ollila@iki.fi">\r
+ <20120202-new-ignore-1-git-send-email-too@iki.fi>\r
+User-Agent: Notmuch/0.11+139~g4340989 (http://notmuchmail.org) Emacs/23.3.1\r
+ (i686-pc-linux-gnu)\r
+Date: Thu, 02 Feb 2012 19:16:03 +0200\r
+Message-ID: <87sjitxijw.fsf@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+Cc: Tomi Ollila <tomi.ollila@iki.fi>\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Thu, 02 Feb 2012 17:16:11 -0000\r
+\r
+\r
+Hi Tomi, please find a few more comments and nitpicks inline. No need to\r
+roll another version for them, though.\r
+\r
+BR,\r
+Jani.\r
+\r
+On Thu, 2 Feb 2012 17:17:33 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:\r
+> A new configuration key 'new.ignore' is used to determine which\r
+> files and directories user wants not to be scanned as new mails.\r
+> \r
+> Mark the corresponding test as no longer broken\r
+> (test from id:"1328105573-4626-1-git-send-email-pieter@praet.org" ).\r
+> \r
+> This work merges my previous attempts and Andreas Amann's work\r
+> in id:"ylp7hi23mw8.fsf@tyndall.ie"\r
+> \r
+> See notes in id:"20120202-new-ignore-1-git-send-email-too@iki.fi"\r
+> ---\r
+> \r
+> Whenever some already existing directory is added to the exclude list\r
+> and the parent directory timestamp has not changed, notmuch new will not\r
+> notice the directory has gone (as it still is there), user needs to 'touch'\r
+> the parent directory before next 'notmuch new' no make notmuch notice.\r
+> \r
+> 2012-01-26: could notmuch track mtime of the configuration file and if\r
+> that changes, ignore mail directory timestamps ?\r
+> \r
+> See previous notes in id:"20120131-new-ignore-1-git-send-email-too@iki.fi"\r
+> \r
+> notmuch-client.h | 10 ++++++++++\r
+> notmuch-config.c | 33 +++++++++++++++++++++++++++++++--\r
+> notmuch-new.c | 45 +++++++++++++++++++++++++++++++++------------\r
+> test/new | 1 -\r
+> 4 files changed, 74 insertions(+), 15 deletions(-)\r
+> \r
+> diff --git a/notmuch-client.h b/notmuch-client.h\r
+> index e0eb594..26153df 100644\r
+> --- a/notmuch-client.h\r
+> +++ b/notmuch-client.h\r
+> @@ -245,11 +245,21 @@ notmuch_config_set_user_other_email (notmuch_config_t *config,\r
+> const char **\r
+> notmuch_config_get_new_tags (notmuch_config_t *config,\r
+> size_t *length);\r
+> +\r
+\r
+Nitpick: unrelated change.\r
+\r
+> void\r
+> notmuch_config_set_new_tags (notmuch_config_t *config,\r
+> const char *new_tags[],\r
+> size_t length);\r
+> \r
+> +const char **\r
+> +notmuch_config_get_new_ignore (notmuch_config_t *config,\r
+> + size_t *length);\r
+> +\r
+> +void\r
+> +notmuch_config_set_new_ignore (notmuch_config_t *config,\r
+> + const char *new_ignore[],\r
+> + size_t length);\r
+> +\r
+> notmuch_bool_t\r
+> notmuch_config_get_maildir_synchronize_flags (notmuch_config_t *config);\r
+> \r
+> diff --git a/notmuch-config.c b/notmuch-config.c\r
+> index a124e34..1f01004 100644\r
+> --- a/notmuch-config.c\r
+> +++ b/notmuch-config.c\r
+> @@ -44,7 +44,10 @@ static const char new_config_comment[] =\r
+> " The following options are supported here:\n"\r
+> "\n"\r
+> "\ttags A list (separated by ';') of the tags that will be\n"\r
+> - "\t added to all messages incorporated by \"notmuch new\".\n";\r
+> + "\t added to all messages incorporated by \"notmuch new\".\n"\r
+> + "\n"\r
+> + "\tignore A list (separated by ';') of file and directory names\n"\r
+> + "\t that will not be searched for messages by \"notmuch new\".\n";\r
+\r
+Do I understand the code correctly, the ignore list must contain file\r
+and directory names without (absolute or relative) paths? And that there\r
+is no way to exclude only the subdirectory "foo" within directory "foo",\r
+because they would both get ignored?\r
+\r
+I don't see this as a bad thing, not at all. This keeps things nice and\r
+simple, it just needs proper documentation.\r
+\r
+> \r
+> static const char user_config_comment[] =\r
+> " User configuration\n"\r
+> @@ -105,6 +108,8 @@ struct _notmuch_config {\r
+> size_t user_other_email_length;\r
+> const char **new_tags;\r
+> size_t new_tags_length;\r
+> + const char **new_ignore;\r
+> + size_t new_ignore_length;\r
+> notmuch_bool_t maildir_synchronize_flags;\r
+> const char **search_exclude_tags;\r
+> size_t search_exclude_tags_length;\r
+> @@ -264,6 +269,8 @@ notmuch_config_open (void *ctx,\r
+> config->user_other_email_length = 0;\r
+> config->new_tags = NULL;\r
+> config->new_tags_length = 0;\r
+> + config->new_ignore = NULL;\r
+> + config->new_ignore_length = 0;\r
+> config->maildir_synchronize_flags = TRUE;\r
+> config->search_exclude_tags = NULL;\r
+> config->search_exclude_tags_length = 0;\r
+> @@ -361,6 +368,10 @@ notmuch_config_open (void *ctx,\r
+> notmuch_config_set_new_tags (config, tags, 2);\r
+> }\r
+> \r
+> + if (notmuch_config_get_new_ignore (config, &tmp) == NULL) {\r
+> + notmuch_config_set_new_ignore (config, NULL, 0);\r
+> + }\r
+> +\r
+> if (notmuch_config_get_search_exclude_tags (config, &tmp) == NULL) {\r
+> if (is_new) {\r
+> const char *tags[] = { "deleted", "spam" };\r
+> @@ -504,7 +515,8 @@ _config_set_list (notmuch_config_t *config,\r
+> const char *list[],\r
+> size_t length, const char ***config_var )\r
+> {\r
+> - g_key_file_set_string_list (config->key_file, group, name, list, length);\r
+> + g_key_file_set_string_list (config->key_file,\r
+> + group, name, list, length);\r
+\r
+Nitpick: unrelated change.\r
+\r
+> talloc_free (*config_var);\r
+> *config_var = NULL;\r
+> }\r
+> @@ -609,6 +621,14 @@ notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)\r
+> &(config->new_tags_length), length);\r
+> }\r
+> \r
+> +const char **\r
+> +notmuch_config_get_new_ignore (notmuch_config_t *config, size_t *length)\r
+> +{\r
+> + return _config_get_list (config, "new", "ignore",\r
+> + &(config->new_ignore),\r
+> + &(config->new_ignore_length), length);\r
+> +}\r
+> +\r
+> void\r
+> notmuch_config_set_user_other_email (notmuch_config_t *config,\r
+> const char *list[],\r
+> @@ -627,6 +647,15 @@ notmuch_config_set_new_tags (notmuch_config_t *config,\r
+> &(config->new_tags));\r
+> }\r
+> \r
+> +void\r
+> +notmuch_config_set_new_ignore (notmuch_config_t *config,\r
+> + const char *list[],\r
+> + size_t length)\r
+> +{\r
+> + _config_set_list (config, "new", "ignore", list, length,\r
+> + &(config->new_ignore));\r
+> +}\r
+> +\r
+> const char **\r
+> notmuch_config_get_search_exclude_tags (notmuch_config_t *config, size_t *length)\r
+> {\r
+> diff --git a/notmuch-new.c b/notmuch-new.c\r
+> index a569a54..f1c271a 100644\r
+> --- a/notmuch-new.c\r
+> +++ b/notmuch-new.c\r
+> @@ -39,6 +39,8 @@ typedef struct {\r
+> int verbose;\r
+> const char **new_tags;\r
+> size_t new_tags_length;\r
+> + const char **new_ignore;\r
+> + size_t new_ignore_length;\r
+> \r
+> int total_files;\r
+> int processed_files;\r
+> @@ -181,6 +183,20 @@ _entries_resemble_maildir (struct dirent **entries, int count)\r
+> return 0;\r
+> }\r
+> \r
+> +/* Test if the file/directory is to be ignored. */\r
+> +\r
+\r
+Nitpick: extra newline.\r
+\r
+> +static notmuch_bool_t\r
+> +_entry_in_ignore_list (const char *entry, add_files_state_t *state)\r
+> +{\r
+> + size_t i;\r
+> +\r
+> + for (i = 0; i < state->new_ignore_length; i++)\r
+> + if (strcmp (entry, state->new_ignore[i]) == 0)\r
+> + return TRUE;\r
+> +\r
+> + return FALSE;\r
+> +}\r
+\r
+Do I understand correctly that if we later allow glob(7) style patterns\r
+in the ignore list, and add the support in place of strcmp(), it will\r
+work with the change in this single place? That would be neat.\r
+\r
+> +\r
+> /* Examine 'path' recursively as follows:\r
+> *\r
+> * o Ask the filesystem for the mtime of 'path' (fs_mtime)\r
+> @@ -320,15 +336,15 @@ add_files_recursive (notmuch_database_t *notmuch,\r
+> }\r
+> \r
+> /* Ignore special directories to avoid infinite recursion.\r
+> - * Also ignore the .notmuch directory and any "tmp" directory\r
+> - * that appears within a maildir.\r
+> + * Also ignore the .notmuch directory, any "tmp" directory\r
+> + * that appears within a maildir and files/directories\r
+> + * the user has configured to be ignored.\r
+> */\r
+> - /* XXX: Eventually we'll want more sophistication to let the\r
+> - * user specify files to be ignored. */\r
+> if (strcmp (entry->d_name, ".") == 0 ||\r
+> strcmp (entry->d_name, "..") == 0 ||\r
+> (is_maildir && strcmp (entry->d_name, "tmp") == 0) ||\r
+> - strcmp (entry->d_name, ".notmuch") ==0)\r
+> + strcmp (entry->d_name, ".notmuch") == 0 ||\r
+> + _entry_in_ignore_list (entry->d_name, state))\r
+\r
+I realize only now that the ignore function *could* also cover the other\r
+ignores, to avoid some code duplication. But that can (and should) be\r
+another refactoring patch, another time, if ever.\r
+\r
+> {\r
+> continue;\r
+> }\r
+> @@ -369,6 +385,10 @@ add_files_recursive (notmuch_database_t *notmuch,\r
+> \r
+> entry = fs_entries[i];\r
+> \r
+> + /* Ignore files & directories user has configured to be ignored */\r
+> + if (_entry_in_ignore_list (entry->d_name, state))\r
+> + continue;\r
+> +\r
+> /* Check if we've walked past any names in db_files or\r
+> * db_subdirs. If so, these have been deleted. */\r
+> while (notmuch_filenames_valid (db_files) &&\r
+> @@ -648,7 +668,7 @@ add_files (notmuch_database_t *notmuch,\r
+> * initialized to zero by the top-level caller before calling\r
+> * count_files). */\r
+> static void\r
+> -count_files (const char *path, int *count)\r
+> +count_files (const char *path, int *count, add_files_state_t *state)\r
+> {\r
+> struct dirent *entry = NULL;\r
+> char *next;\r
+> @@ -670,13 +690,13 @@ count_files (const char *path, int *count)\r
+> entry = fs_entries[i++];\r
+> \r
+> /* Ignore special directories to avoid infinite recursion.\r
+> - * Also ignore the .notmuch directory.\r
+> + * Also ignore the .notmuch directory and files/directories\r
+> + * the user has configured to be ignored.\r
+> */\r
+> - /* XXX: Eventually we'll want more sophistication to let the\r
+> - * user specify files to be ignored. */\r
+> if (strcmp (entry->d_name, ".") == 0 ||\r
+> strcmp (entry->d_name, "..") == 0 ||\r
+> - strcmp (entry->d_name, ".notmuch") == 0)\r
+> + strcmp (entry->d_name, ".notmuch") == 0 ||\r
+> + _entry_in_ignore_list (entry->d_name, state))\r
+> {\r
+> continue;\r
+> }\r
+> @@ -697,7 +717,7 @@ count_files (const char *path, int *count)\r
+> fflush (stdout);\r
+> }\r
+> } else if (S_ISDIR (st.st_mode)) {\r
+> - count_files (next, count);\r
+> + count_files (next, count, state);\r
+> }\r
+> \r
+> free (next);\r
+> @@ -837,6 +857,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])\r
+> return 1;\r
+> \r
+> add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length);\r
+> + add_files_state.new_ignore = notmuch_config_get_new_ignore (config, &add_files_state.new_ignore_length);\r
+> add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);\r
+> db_path = notmuch_config_get_database_path (config);\r
+> \r
+> @@ -852,7 +873,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])\r
+> int count;\r
+> \r
+> count = 0;\r
+> - count_files (db_path, &count);\r
+> + count_files (db_path, &count, &add_files_state);\r
+> if (interrupted)\r
+> return 1;\r
+> \r
+> diff --git a/test/new b/test/new\r
+> index 740ba05..b616dec 100755\r
+> --- a/test/new\r
+> +++ b/test/new\r
+> @@ -166,7 +166,6 @@ Note: Ignoring non-mail file: ${MAIL_DIR}/ignored_file\r
+> Added 1 new message to the database."\r
+> \r
+> test_begin_subtest "Ignore files and directories specified in new.ignore"\r
+> -test_subtest_known_broken\r
+> generate_message\r
+> notmuch config set new.ignore .git ignored_file .ignored_hidden_file\r
+> mkdir -p "${MAIL_DIR}"/.git && touch "${MAIL_DIR}"/.git/config\r
+> -- \r
+> 1.7.8.2\r
+> \r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r
projects / notmuch-archives.git / commitdiff