From: Jani Nikula <jani@nikula.org>
To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Cc: Tomi Ollila <tomi.ollila@iki.fi>
Subject: Re: [PATCH] added support for user-specified files & directories to ignore
Date: Wed, 01 Feb 2012 14:49:45 +0000 [thread overview]
Message-ID: <87d39y622e.fsf@nikula.org> (raw)
In-Reply-To: <20120131-new-ignore-1-git-send-email-too@iki.fi>
On Tue, 31 Jan 2012 18:28:04 +0200, Tomi Ollila <tomi.ollila@iki.fi> 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
next prev parent reply other threads:[~2012-02-01 14:49 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-13 21:32 [PATCH 0/3] Configuration file option to exclude files/directories tomi.ollila
2011-09-13 21:32 ` [PATCH 1/3] added function notmuch_talloc_g_key_file_get_string_list() tomi.ollila
2011-09-13 21:32 ` [PATCH 2/3] Made notmuch_config_get_new_tags() use notmuch_talloc_g_key_file_get_string_list() tomi.ollila
2011-09-13 21:32 ` [PATCH 3/3] added support for user-specified directories to exclude tomi.ollila
2011-12-07 23:51 ` [PATCH 0/3] Configuration file option to exclude files/directories David Bremner
2011-12-08 0:24 ` Tom Prince
2011-12-08 9:05 ` Tomi Ollila
2012-01-25 0:46 ` Pieter Praet
2012-01-26 10:11 ` [PATCH 1/2] moved _notmuch_get_list () and _notmuch_set_list () up in file Tomi Ollila
2012-01-26 10:11 ` [PATCH 2/2] added support for user-specified directories to exclude Tomi Ollila
2012-01-26 13:11 ` Jani Nikula
2012-01-27 10:41 ` Tomi Ollila
2012-01-27 22:14 ` Austin Clements
2012-01-28 14:16 ` Fabio Zanini
2012-01-27 22:50 ` Austin Clements
2012-01-26 13:03 ` [PATCH 1/2] moved _notmuch_get_list () and _notmuch_set_list () up in file Jani Nikula
2012-01-27 10:42 ` Tomi Ollila
2012-01-27 18:18 ` Ethan Glasser-Camp
2012-01-30 10:31 ` [PATCH] moved _config_(get|set)_list () functions earlyer in the file Tomi Ollila
2012-01-30 15:06 ` Austin Clements
2012-01-31 3:28 ` David Bremner
2012-01-27 10:59 ` [PATCH] moved _notmuch_(get|set)_list " Tomi Ollila
2012-01-27 22:15 ` Austin Clements
2012-01-25 6:28 ` [PATCH 0/3] Configuration file option to exclude files/directories David Edmondson
2012-02-01 14:12 ` [PATCH] test: add tests wrt ignoring user-specified files and directories Pieter Praet
2012-02-03 12:14 ` Tomi Ollila
2012-02-03 22:39 ` Austin Clements
2012-02-03 22:44 ` Austin Clements
2012-02-19 20:43 ` Pieter Praet
2012-01-31 16:28 ` [PATCH] added support for user-specified files & directories to ignore Tomi Ollila
2012-02-01 14:25 ` Pieter Praet
2012-02-01 14:49 ` Jani Nikula [this message]
2012-02-01 16:30 ` Austin Clements
2012-02-06 9:28 ` [PATCH v6 1/3] test: add tests wrt ignoring user-specified files and directories Tomi Ollila
2012-02-06 9:28 ` [PATCH v6 2/3] add support for user-specified files & directories to ignore Tomi Ollila
2012-02-06 9:50 ` Tomi Ollila
2012-02-06 9:28 ` [PATCH v6 3/3] NEWS: add news section for new.ignore Tomi Ollila
2012-02-14 19:18 ` Austin Clements
2012-02-13 8:20 ` [PATCH v6 1/3] test: add tests wrt ignoring user-specified files and directories Tomi Ollila
2012-02-14 19:20 ` Austin Clements
2012-02-14 19:18 ` Austin Clements
2012-02-15 9:17 ` [PATCH v7 0/3] NEWS and test comment adjustments Tomi Ollila
2012-02-15 9:17 ` [PATCH v7 1/3] test: add tests wrt ignoring user-specified files and directories Tomi Ollila
2012-02-15 9:17 ` [PATCH v7 2/3] add support for user-specified files & directories to ignore Tomi Ollila
2012-02-15 9:17 ` [PATCH v7 3/3] NEWS: add news section for new.ignore Tomi Ollila
2012-02-17 12:13 ` [PATCH v7 0/3] NEWS and test comment adjustments David Bremner
2012-02-19 20:47 ` Pieter Praet
2012-02-19 20:47 ` [PATCH 1/6] cli: update 'new.ignore' config file comment wrt file/directory matching Pieter Praet
2012-02-28 2:46 ` David Bremner
2012-02-19 20:47 ` [PATCH 2/6] NEWS: sync 'new.ignore' entry with its comment in notmuch-config.c Pieter Praet
2012-02-28 2:46 ` David Bremner
2012-02-19 20:47 ` [PATCH 3/6] cli: add '--debug' option to 'notmuch new' Pieter Praet
2012-02-19 20:47 ` [PATCH 4/6] cli: notmuch new: optionally output debug information when ignoring files/directories Pieter Praet
2012-02-19 20:47 ` [PATCH 5/6] test-lib.sh: pass 'NOTMUCH_NEW's args down to 'notmuch new' Pieter Praet
2012-02-19 20:47 ` [PATCH 6/6] test: another test wrt ignoring user-specified files and directories Pieter Praet
2012-08-31 21:13 ` David Bremner
2012-10-12 19:32 ` [PATCH] " Ethan Glasser-Camp
2012-10-20 20:42 ` David Bremner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87d39y622e.fsf@nikula.org \
--to=jani@nikula.org \
--cc=notmuch@notmuchmail.org \
--cc=tomi.ollila@iki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).