* Re: [PATCH] add support for user-specified files & directories to ignore
2012-02-02 15:17 ` [PATCH] add support for user-specified files & directories to ignore Tomi Ollila
@ 2012-02-02 17:16 ` Jani Nikula
2012-02-02 17:57 ` [PATCH v5] " Tomi Ollila
1 sibling, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2012-02-02 17:16 UTC (permalink / raw)
To: Tomi Ollila, notmuch; +Cc: Tomi Ollila
Hi Tomi, please find a few more comments and nitpicks inline. No need to
roll another version for them, though.
BR,
Jani.
On Thu, 2 Feb 2012 17:17:33 +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.
>
> Mark the corresponding test as no longer broken
> (test from id:"1328105573-4626-1-git-send-email-pieter@praet.org" ).
>
> This work merges my previous attempts and Andreas Amann's work
> in id:"ylp7hi23mw8.fsf@tyndall.ie"
>
> See notes in id:"20120202-new-ignore-1-git-send-email-too@iki.fi"
> ---
>
> 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 ?
>
> See previous notes in id:"20120131-new-ignore-1-git-send-email-too@iki.fi"
>
> notmuch-client.h | 10 ++++++++++
> notmuch-config.c | 33 +++++++++++++++++++++++++++++++--
> notmuch-new.c | 45 +++++++++++++++++++++++++++++++++------------
> test/new | 1 -
> 4 files changed, 74 insertions(+), 15 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index e0eb594..26153df 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -245,11 +245,21 @@ notmuch_config_set_user_other_email (notmuch_config_t *config,
> const char **
> notmuch_config_get_new_tags (notmuch_config_t *config,
> size_t *length);
> +
Nitpick: unrelated change.
> void
> 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..1f01004 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 file and directory names\n"
> + "\t that will not be searched for messages by \"notmuch new\".\n";
Do I understand the code correctly, the ignore list must contain file
and directory names without (absolute or relative) paths? And that there
is no way to exclude only the subdirectory "foo" within directory "foo",
because they would both get ignored?
I don't see this as a bad thing, not at all. This keeps things nice and
simple, it just needs proper documentation.
>
> 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;
> @@ -361,6 +368,10 @@ notmuch_config_open (void *ctx,
> notmuch_config_set_new_tags (config, tags, 2);
> }
>
> + if (notmuch_config_get_new_ignore (config, &tmp) == NULL) {
> + notmuch_config_set_new_ignore (config, NULL, 0);
> + }
> +
> if (notmuch_config_get_search_exclude_tags (config, &tmp) == NULL) {
> if (is_new) {
> const char *tags[] = { "deleted", "spam" };
> @@ -504,7 +515,8 @@ _config_set_list (notmuch_config_t *config,
> const char *list[],
> size_t length, const char ***config_var )
> {
> - g_key_file_set_string_list (config->key_file, group, name, list, length);
> + g_key_file_set_string_list (config->key_file,
> + group, name, list, length);
Nitpick: unrelated change.
> talloc_free (*config_var);
> *config_var = NULL;
> }
> @@ -609,6 +621,14 @@ 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)
> +{
> + 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,15 @@ notmuch_config_set_new_tags (notmuch_config_t *config,
> &(config->new_tags));
> }
>
> +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));
> +}
> +
> 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..f1c271a 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;
> }
>
> +/* Test if the file/directory is to be ignored. */
> +
Nitpick: extra newline.
> +static notmuch_bool_t
> +_entry_in_ignore_list (const char *entry, add_files_state_t *state)
> +{
> + size_t i;
> +
> + for (i = 0; i < state->new_ignore_length; i++)
> + if (strcmp (entry, state->new_ignore[i]) == 0)
> + return TRUE;
> +
> + return FALSE;
> +}
Do I understand correctly that if we later allow glob(7) style patterns
in the ignore list, and add the support in place of strcmp(), it will
work with the change in this single place? That would be neat.
> +
> /* 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
> + * the user has configured to be ignored.
> */
> - /* 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))
I realize only now that the ignore function *could* also cover the other
ignores, to avoid some code duplication. But that can (and should) be
another refactoring patch, another time, if ever.
> {
> 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
> + * the user has configured to be ignored.
> */
> - /* 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 ||
> - 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;
>
> diff --git a/test/new b/test/new
> index 740ba05..b616dec 100755
> --- a/test/new
> +++ b/test/new
> @@ -166,7 +166,6 @@ Note: Ignoring non-mail file: ${MAIL_DIR}/ignored_file
> Added 1 new message to the database."
>
> test_begin_subtest "Ignore files and directories specified in new.ignore"
> -test_subtest_known_broken
> generate_message
> notmuch config set new.ignore .git ignored_file .ignored_hidden_file
> mkdir -p "${MAIL_DIR}"/.git && touch "${MAIL_DIR}"/.git/config
> --
> 1.7.8.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v5] add support for user-specified files & directories to ignore
2012-02-02 15:17 ` [PATCH] add support for user-specified files & directories to ignore Tomi Ollila
2012-02-02 17:16 ` Jani Nikula
@ 2012-02-02 17:57 ` Tomi Ollila
2012-02-03 14:56 ` Pieter Praet
` (2 more replies)
1 sibling, 3 replies; 6+ messages in thread
From: Tomi Ollila @ 2012-02-02 17:57 UTC (permalink / raw)
To: notmuch; +Cc: Tomi Ollila
A new configuration key 'new.ignore' is used to determine which
files and directories user wants not to be scanned as new mails.
Mark the corresponding test as no longer broken
(test from id:"1328105573-4626-1-git-send-email-pieter@praet.org" ).
This work merges my previous attempts and Andreas Amann's work
in id:"ylp7hi23mw8.fsf@tyndall.ie"
See notes in id:"20120202-new-ignore-1-git-send-email-too@iki.fi"
---
notmuch-client.h | 9 +++++++++
notmuch-config.c | 30 +++++++++++++++++++++++++++++-
notmuch-new.c | 45 +++++++++++++++++++++++++++++++++------------
test/new | 1 -
4 files changed, 71 insertions(+), 14 deletions(-)
diff --git a/notmuch-client.h b/notmuch-client.h
index e0eb594..f1762ae 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -250,6 +250,15 @@ 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..1f01128 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 file and directory names\n"
+ "\t that 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;
@@ -361,6 +368,10 @@ notmuch_config_open (void *ctx,
notmuch_config_set_new_tags (config, tags, 2);
}
+ if (notmuch_config_get_new_ignore (config, &tmp) == NULL) {
+ notmuch_config_set_new_ignore (config, NULL, 0);
+ }
+
if (notmuch_config_get_search_exclude_tags (config, &tmp) == NULL) {
if (is_new) {
const char *tags[] = { "deleted", "spam" };
@@ -609,6 +620,14 @@ 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)
+{
+ 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 +646,15 @@ notmuch_config_set_new_tags (notmuch_config_t *config,
&(config->new_tags));
}
+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));
+}
+
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..8a615e6 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;
}
+/* Test if the file/directory is to be ignored.
+ */
+static notmuch_bool_t
+_entry_in_ignore_list (const char *entry, add_files_state_t *state)
+{
+ size_t i;
+
+ for (i = 0; i < state->new_ignore_length; i++)
+ if (strcmp (entry, state->new_ignore[i]) == 0)
+ return TRUE;
+
+ return FALSE;
+}
+
/* 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
+ * the user has configured to be ignored.
*/
- /* 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
+ * the user has configured to be ignored.
*/
- /* 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 ||
- 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;
diff --git a/test/new b/test/new
index 740ba05..b616dec 100755
--- a/test/new
+++ b/test/new
@@ -166,7 +166,6 @@ Note: Ignoring non-mail file: ${MAIL_DIR}/ignored_file
Added 1 new message to the database."
test_begin_subtest "Ignore files and directories specified in new.ignore"
-test_subtest_known_broken
generate_message
notmuch config set new.ignore .git ignored_file .ignored_hidden_file
mkdir -p "${MAIL_DIR}"/.git && touch "${MAIL_DIR}"/.git/config
--
1.7.8.2
^ permalink raw reply related [flat|nested] 6+ messages in thread