From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id D401D429E5B for ; Wed, 1 Feb 2012 06:49:53 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 5jdVGFK0o9U6 for ; Wed, 1 Feb 2012 06:49:52 -0800 (PST) Received: from mail-qy0-f181.google.com (mail-qy0-f181.google.com [209.85.216.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id CB78E429E34 for ; Wed, 1 Feb 2012 06:49:51 -0800 (PST) Received: by qcsd17 with SMTP id d17so814686qcs.26 for ; Wed, 01 Feb 2012 06:49:50 -0800 (PST) Received: by 10.229.106.70 with SMTP id w6mr10238782qco.132.1328107789811; Wed, 01 Feb 2012 06:49:49 -0800 (PST) Received: from localhost (nikula.org. [92.243.24.172]) by mx.google.com with ESMTPS id dm7sm48023966qab.5.2012.02.01.06.49.46 (version=SSLv3 cipher=OTHER); Wed, 01 Feb 2012 06:49:48 -0800 (PST) From: Jani Nikula To: Tomi Ollila , notmuch@notmuchmail.org Subject: Re: [PATCH] added support for user-specified files & directories to ignore In-Reply-To: <20120131-new-ignore-1-git-send-email-too@iki.fi> References: <1315949524-4948-1-git-send-email-tomi.ollila@iki.fi> <20120131-new-ignore-1-git-send-email-too@iki.fi> User-Agent: Notmuch/0.10.2+187~g43d4f26 (http://notmuchmail.org) Emacs/23.1.1 (i686-pc-linux-gnu) Date: Wed, 01 Feb 2012 14:49:45 +0000 Message-ID: <87d39y622e.fsf@nikula.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Tomi Ollila X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Feb 2012 14:49:54 -0000 On Tue, 31 Jan 2012 18:28:04 +0200, Tomi Ollila wrote: > A new configuration key 'new.ignore' is used to determine which > files and directories user wants not to be scanned as new mails. Hi, please find a few drive-by "while my code is compiling" nitpick review comments inline. ;) Mostly looks good. BR, Jani. > > This work merges my previous attempts and Andreas Amann's work > in id:"ylp7hi23mw8.fsf@tyndall.ie" > > See notes in id:"20120131-new-ignore-1-git-send-email-too@iki.fi" > --- > Notes > > 1) Currently there is comment for new.ignore in newly created configuration > file but as the list is initially empty there will be not tag in place. > > 2) Whenever some already existing directory is added to the exclude list > and the parent directory timestamp has not changed, notmuch new will not > notice the directory has gone (as it still is there), user needs to 'touch' > the parent directory before next 'notmuch new' no make notmuch notice. > > 2012-01-26: could notmuch track mtime of the configuration file and if > that changes, ignore mail directory timestamps ? > > > 3) in id:"1327572718-13411-2-git-send-email-tomi.ollila@iki.fi" dropped... > > notmuch-client.h | 8 ++++++++ > notmuch-config.c | 35 +++++++++++++++++++++++++++++++++-- > notmuch-new.c | 45 +++++++++++++++++++++++++++++++++------------ > 3 files changed, 74 insertions(+), 14 deletions(-) > > diff --git a/notmuch-client.h b/notmuch-client.h > index e0eb594..c62ce78 100644 > --- a/notmuch-client.h > +++ b/notmuch-client.h > @@ -250,6 +250,14 @@ notmuch_config_set_new_tags (notmuch_config_t *config, > const char *new_tags[], > size_t length); > > +const char ** > +notmuch_config_get_new_ignore (notmuch_config_t *config, > + size_t *length); > +void > +notmuch_config_set_new_ignore (notmuch_config_t *config, > + const char *new_ignore[], > + size_t length); > + > notmuch_bool_t > notmuch_config_get_maildir_synchronize_flags (notmuch_config_t *config); > > diff --git a/notmuch-config.c b/notmuch-config.c > index a124e34..f1cc5c2 100644 > --- a/notmuch-config.c > +++ b/notmuch-config.c > @@ -44,7 +44,10 @@ static const char new_config_comment[] = > " The following options are supported here:\n" > "\n" > "\ttags A list (separated by ';') of the tags that will be\n" > - "\t added to all messages incorporated by \"notmuch new\".\n"; > + "\t added to all messages incorporated by \"notmuch new\".\n" > + "\n" > + "\tignore A list (separated by ';') of files and directories that" > + "\t will not be searched for messages by \"notmuch new\".\n"; > > static const char user_config_comment[] = > " User configuration\n" > @@ -105,6 +108,8 @@ struct _notmuch_config { > size_t user_other_email_length; > const char **new_tags; > size_t new_tags_length; > + const char **new_ignore; > + size_t new_ignore_length; > notmuch_bool_t maildir_synchronize_flags; > const char **search_exclude_tags; > size_t search_exclude_tags_length; > @@ -264,6 +269,8 @@ notmuch_config_open (void *ctx, > config->user_other_email_length = 0; > config->new_tags = NULL; > config->new_tags_length = 0; > + config->new_ignore = NULL; > + config->new_ignore_length = 0; > config->maildir_synchronize_flags = TRUE; > config->search_exclude_tags = NULL; > config->search_exclude_tags_length = 0; > @@ -360,7 +367,11 @@ notmuch_config_open (void *ctx, > const char *tags[] = { "unread", "inbox" }; > notmuch_config_set_new_tags (config, tags, 2); > } > - > +#if 0 /* No point setting empty list -- it's not written */ > + if (notmuch_config_get_new_ignore (config, &tmp) == NULL) { > + notmuch_config_set_new_ignore (config, NULL, 0); > + } > +#endif I'd prefer not having any #if 0 code. How about just having the code there, even if it ends up being a NOP? You never know when the config plumbing is going to change (it could use some love), e.g. it might write the default value in comments in the future. > if (notmuch_config_get_search_exclude_tags (config, &tmp) == NULL) { > if (is_new) { > const char *tags[] = { "deleted", "spam" }; > @@ -609,6 +620,15 @@ notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length) > &(config->new_tags_length), length); > } > > +const char ** > +notmuch_config_get_new_ignore (notmuch_config_t *config, size_t *length) Is there extra space in there? > +{ > + return _config_get_list (config, "new", "ignore", > + &(config->new_ignore), > + &(config->new_ignore_length), length); > +} > + > + > void > notmuch_config_set_user_other_email (notmuch_config_t *config, > const char *list[], > @@ -627,6 +647,17 @@ notmuch_config_set_new_tags (notmuch_config_t *config, > &(config->new_tags)); > } > > +#if 0 /* UNNEEDED SO FAR */ > +void > +notmuch_config_set_new_ignore (notmuch_config_t *config, > + const char *list[], > + size_t length) > +{ > + _config_set_list (config, "new", "ignore", list, length, > + &(config->new_ignore)); > +} > +#endif Same here, just remove the #if 0. > + > const char ** > notmuch_config_get_search_exclude_tags (notmuch_config_t *config, size_t *length) > { > diff --git a/notmuch-new.c b/notmuch-new.c > index a569a54..36d5c5d 100644 > --- a/notmuch-new.c > +++ b/notmuch-new.c > @@ -39,6 +39,8 @@ typedef struct { > int verbose; > const char **new_tags; > size_t new_tags_length; > + const char **new_ignore; > + size_t new_ignore_length; > > int total_files; > int processed_files; > @@ -181,6 +183,20 @@ _entries_resemble_maildir (struct dirent **entries, int count) > return 0; > } > > +/* Check if user asked to ignore these files/directories */ > + > +static int notmuch_bool_t? > +_entry_in_ignore_list (const char *entry, add_files_state_t *state) > +{ > + size_t i, ignore_length = state->new_ignore_length; ignore_length is an unnecessary temp variable. > + > + for (i = 0; i < ignore_length; i++) > + if (strcmp (entry, state->new_ignore[i]) == 0) > + return 1; > + > + return 0; > +} > + > /* Examine 'path' recursively as follows: > * > * o Ask the filesystem for the mtime of 'path' (fs_mtime) > @@ -320,15 +336,15 @@ add_files_recursive (notmuch_database_t *notmuch, > } > > /* Ignore special directories to avoid infinite recursion. > - * Also ignore the .notmuch directory and any "tmp" directory > - * that appears within a maildir. > + * Also ignore the .notmuch directory, any "tmp" directory > + * that appears within a maildir and files/directories > + * user have configured to be ignored. s/user have/the user has/ ? > */ > - /* XXX: Eventually we'll want more sophistication to let the > - * user specify files to be ignored. */ > if (strcmp (entry->d_name, ".") == 0 || > strcmp (entry->d_name, "..") == 0 || > (is_maildir && strcmp (entry->d_name, "tmp") == 0) || > - strcmp (entry->d_name, ".notmuch") ==0) > + strcmp (entry->d_name, ".notmuch") == 0 || > + _entry_in_ignore_list (entry->d_name, state)) > { > continue; > } > @@ -369,6 +385,10 @@ add_files_recursive (notmuch_database_t *notmuch, > > entry = fs_entries[i]; > > + /* Ignore files & directories user has configured to be ignored */ > + if (_entry_in_ignore_list (entry->d_name, state)) > + continue; > + > /* Check if we've walked past any names in db_files or > * db_subdirs. If so, these have been deleted. */ > while (notmuch_filenames_valid (db_files) && > @@ -648,7 +668,7 @@ add_files (notmuch_database_t *notmuch, > * initialized to zero by the top-level caller before calling > * count_files). */ > static void > -count_files (const char *path, int *count) > +count_files (const char *path, int *count, add_files_state_t *state) > { > struct dirent *entry = NULL; > char *next; > @@ -670,13 +690,13 @@ count_files (const char *path, int *count) > entry = fs_entries[i++]; > > /* Ignore special directories to avoid infinite recursion. > - * Also ignore the .notmuch directory. > + * Also ignore the .notmuch directory and files/directories > + * user have configured to be ignored. Same as above. > */ > - /* XXX: Eventually we'll want more sophistication to let the > - * user specify files to be ignored. */ Oh, you think this is enough sophistication! ;) > if (strcmp (entry->d_name, ".") == 0 || > strcmp (entry->d_name, "..") == 0 || > - strcmp (entry->d_name, ".notmuch") == 0) > + strcmp (entry->d_name, ".notmuch") == 0 || > + _entry_in_ignore_list (entry->d_name, state)) > { > continue; > } > @@ -697,7 +717,7 @@ count_files (const char *path, int *count) > fflush (stdout); > } > } else if (S_ISDIR (st.st_mode)) { > - count_files (next, count); > + count_files (next, count, state); > } > > free (next); > @@ -837,6 +857,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[]) > return 1; > > add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length); > + add_files_state.new_ignore = notmuch_config_get_new_ignore (config, &add_files_state.new_ignore_length); > add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); > db_path = notmuch_config_get_database_path (config); > > @@ -852,7 +873,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[]) > int count; > > count = 0; > - count_files (db_path, &count); > + count_files (db_path, &count, &add_files_state); > if (interrupted) > return 1; > > -- > 1.7.6.5 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch