[PATCH 0/4] Make tags applied by 'notmuch new' configurable.

[PATCH 0/4] Make tags applied by 'notmuch new' configurable.

[Jan Janak]

I would like to propose that we make the list of tags applied by 'notmuch new'
configurable. Right now notmuch applies two tags to all new messages added to
the database, 'inbox' and 'unread'. The two tags are added by the C code in
notmuch-new.c and they cannot be changed without editing the source file and
recompiling notmuch.

The four patches that follow this email allow for configuring the tags to be
added by 'notmuch new' either in the configuration file or on the command
line.

This change was motivated by my desire to remove both tags from newly added
messages. My rules for adding these two tags are more complex and I do it in
a script run after 'notmuch new'. Instead of 'inbox' and 'unread', I configure
'notmuch new' to add a new tag called 'new' (and only that one). This tag
marks newly added messages that haven't been properly tagged yet by my 
auto-tagging scripts. The last script I run after 'notmuch new' removes that
tag. My auto-tagging scripts process only messages with the tag 'new'.

On a side note; It may seem logical to add/omit the tag 'unread' directly in 
'notmuch new' based on the Maildir flags extracted from the filename of the
message. I suggest that we don't do that in 'notmuch new'. A better way would
be writing a small script or command that can be run *after* 'notmuch new'.
We could then distribute it with notmuch (maybe with other small tagging
scripts for common situations). 

I think Maildir flags should be processed after 'notmuch new' is because if
there are multiple copies of a message with different flags, we may need to
see all flags from all filenames to set corresponding tags properly and we may
also need to take the directory part into consideration (i.e. the new mail is
in 'new', not 'cur').

The list of tags to be applied by notmuch can be configured in the
configuration file. There is a new section [new] which contains configuration
options for 'notmuch new'. There is only one option called 'tags'. The option
contains a semicolon separated list of tags:

  [new]
  tags=inbox;unread  # Emulate the original behavior

One of the patches updates 'notmuch setup' to create the section and add
the tags option with tags 'inbox' and 'unread', but only if a new
configuration file is being created. If the configuration file already exists
then it just copies the contents from the old configuration file to the new
one.

We do not ask the user for the list of tags in the interactive part, that would
have been too much. Users can edit the configuration file manually if they want
to change the list of tags. If they create a new configuration file then they
probably want to accept the default anyway.

There is one catch for users who already have a configuration file and start
using the patches. They will need to add the new section and the tags option
manually if they want to preserve current behavior of applying 'inbox' and
'unread' automatically by 'notmuch new'.

The last patch in the set adds a new command line option to 'notmuch new'.
The name of the option is --tag and it can be used to override any tags
configured in the configuration file. For example:

  notmuch new --tag=outbox --tag=read

adds the tags 'outbox' and 'read' and ignores any tags from the configuration
file.

Comments and opinions are welcome!

   -- Jan

[PATCH 1/4] notmuch-new: Remove tag_add_inbox_unread in favor of a generic solution.

[Jan Janak]

Instead of adding 'inbox' and 'unread' tags directly in the code of
'notmuch-new', we can specify a list of tags to be added to newly
created messages with a configuration file option or a command line
option. That's more flexible, it allows the user to select which tags
should be added.

Signed-off-by: Jan Janak <jan@ryngle.com>
---
 notmuch-new.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index e32b92a..9970407 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -41,13 +41,6 @@ handle_sigint (unused (int sig))
 }
 
 static void
-tag_inbox_and_unread (notmuch_message_t *message)
-{
-    notmuch_message_add_tag (message, "inbox");
-    notmuch_message_add_tag (message, "unread");
-}
-
-static void
 add_files_print_progress (add_files_state_t *state)
 {
     struct timeval tv_now;
@@ -198,7 +191,6 @@ add_files_recursive (notmuch_database_t *notmuch,
 		    /* success */
 		    case NOTMUCH_STATUS_SUCCESS:
 			state->added_messages++;
-			tag_inbox_and_unread (message);
 			break;
 		    /* Non-fatal issues (go on to next file) */
 		    case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
-- 
1.6.3.3

[PATCH 2/4] notmuch: Config option to specify tags to be applied by 'notmuch new'.

[Jan Janak]

Add support for section [new] in the configuration file. This section
is supposed to contain options for 'notmuch new'. Currently there is
only one option called tags.

The tags option can be used to configure a set of tags to be applied
by 'notmuch new'. Individual tags are separated by semicolon.

'notmuch new' is modified not to apply 'inbox' and 'unread' by default,
but instead it obtains the set of tags to be applied from the new
configuration file option.

Signed-off-by: Jan Janak <jan@ryngle.com>
---
 notmuch-client.h |    3 +++
 notmuch-config.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 notmuch-new.c    |   19 ++++++++++++++++++-
 3 files changed, 65 insertions(+), 1 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index c04eaeb..0fb9c19 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -184,6 +184,9 @@ notmuch_config_set_user_other_email (notmuch_config_t *config,
 				     const char *other_email[],
 				     size_t length);
 
+char **
+notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);
+
 notmuch_bool_t
 debugger_is_active (void);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index fc65d6b..7f62a80 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -22,6 +22,7 @@
 
 #include <pwd.h>
 #include <netdb.h>
+#include <ctype.h>
 
 static const char toplevel_config_comment[] =
     " .notmuch-config - Configuration file for the notmuch mail system\n"
@@ -62,6 +63,9 @@ struct _notmuch_config {
     char *user_primary_email;
     char **user_other_email;
     size_t user_other_email_length;
+
+    char **new_tags;
+    size_t new_tags_length;
 };
 
 static int
@@ -199,6 +203,8 @@ notmuch_config_open (void *ctx,
     config->user_primary_email = NULL;
     config->user_other_email = NULL;
     config->user_other_email_length = 0;
+    config->new_tags = NULL;
+    config->new_tags_length = 0;
 
     if (! g_key_file_load_from_file (config->key_file,
 				     config->filename,
@@ -450,3 +456,41 @@ notmuch_config_set_user_other_email (notmuch_config_t *config,
     talloc_free (config->user_other_email);
     config->user_other_email = NULL;
 }
+
+char **
+notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)
+{
+    char **tags;
+    size_t len;
+    unsigned int i;
+    char *start, *end;
+
+    if (config->new_tags == NULL) {
+	config->new_tags_length = 0;
+	tags = g_key_file_get_string_list (config->key_file, "new", "tags",
+					   &len, NULL);
+
+	if (tags) {
+	    config->new_tags = talloc_size (config, sizeof(char*) *
+					    (len + 1));
+	    for (i = 0; i < len; i++) {
+		/* Remove leading and trailing white space around the tag and
+		 * filter out empty tags. */
+		start = tags[i];
+		end = start + strlen (start) - 1;
+		while (isspace (*start)) start++;
+		while (end > start && isspace (*end)) end--;
+		if (end >= start) {
+		    config->new_tags[config->new_tags_length++] =
+			talloc_strndup (config->new_tags, start,
+					end - start + 1);
+		}
+	    }
+	    config->new_tags[config->new_tags_length] = NULL;
+	    g_strfreev (tags);
+	}
+    }
+
+    *length = config->new_tags_length;
+    return config->new_tags;
+}
diff --git a/notmuch-new.c b/notmuch-new.c
index 9970407..10745e8 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -24,6 +24,8 @@
 
 static volatile sig_atomic_t do_add_files_print_progress = 0;
 
+static notmuch_config_t *config = NULL;
+
 static void
 handle_sigalrm (unused (int signal))
 {
@@ -68,6 +70,21 @@ add_files_print_progress (add_files_state_t *state)
     fflush (stdout);
 }
 
+static void
+apply_tags (notmuch_message_t *message)
+{
+    char** tags;
+    unsigned int count, i;
+
+    if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
+	return;
+
+    for (i = 0; i < count; i++) {
+	if (tags[i])
+	    notmuch_message_add_tag (message, tags[i]);
+    }
+}
+
 static int ino_cmp(const struct dirent **a, const struct dirent **b)
 {
     return ((*a)->d_ino < (*b)->d_ino) ? -1 : 1;
@@ -191,6 +208,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 		    /* success */
 		    case NOTMUCH_STATUS_SUCCESS:
 			state->added_messages++;
+			apply_tags (message);
 			break;
 		    /* Non-fatal issues (go on to next file) */
 		    case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
@@ -388,7 +406,6 @@ count_files (const char *path, int *count)
 int
 notmuch_new_command (void *ctx, int argc, char *argv[])
 {
-    notmuch_config_t *config;
     notmuch_database_t *notmuch;
     add_files_state_t add_files_state;
     double elapsed;
-- 
1.6.3.3

[PATCH 3/4] notmuch-setup: Copy/create the new section with tags for 'notmuch-new'.

[Jan Janak]

If the user runs 'notmuch setup' and there is no configuration file yet
then we also add the new section [new] to the configuration file and
set the tags option as:

  tags=inbox;unread

This will be picked up by 'notmuch new' and all new mail added to the
database will be tagged with the two tags as before.

If the user already has a configuration file and runs 'notmuch setup'
then we just copy whatever tags we find in the old configuration file
to the new one. If there are no tags in the old configuration file then
we assume that the user configured notmuch that way and the new config
file would also have no tags in the section [new].

We never ask the user interactively for the list of tags to be used by
'notmuch new', it is assumed that beginners would want to stick to the
defaults and advanced users can edit the configuration manually.

Signed-off-by: Jan Janak <jan@ryngle.com>
---
 notmuch-client.h |    4 ++++
 notmuch-config.c |   40 ++++++++++++++++++++++++++++++++++++++++
 notmuch-setup.c  |   14 ++++++++++++++
 3 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 0fb9c19..bb7d3d4 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -187,6 +187,10 @@ notmuch_config_set_user_other_email (notmuch_config_t *config,
 char **
 notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);
 
+void
+notmuch_config_set_new_tags (notmuch_config_t *config, const char *tags[],
+			     size_t length);
+
 notmuch_bool_t
 debugger_is_active (void);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index 7f62a80..e884621 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -54,6 +54,16 @@ static const char user_config_comment[] =
     " recipient list of replies, and will set the From address based on the\n"
     " address to which the original email was addressed.\n";
 
+static const char new_config_comment[] =
+    " Configuration section for 'notmuch new'\n"
+    "\n"
+    " The only supported value at the moment is 'tags. This option contains a\n"
+    " list of tags (separated by ';') that should be  automatically applied to\n"
+    " newly added messages.\n"
+    "\n"
+    " Note that 'notmuch new' also has a command line option which can be used\n"
+    " to add additional tags to the ones configured here.\n";
+
 struct _notmuch_config {
     char *filename;
     GKeyFile *key_file;
@@ -174,6 +184,7 @@ notmuch_config_open (void *ctx,
     GError *error = NULL;
     int is_new = 0;
     char *notmuch_config_env = NULL;
+    const char* def_new_tags[2] = {"inbox", "unread"};
 
     if (is_new_ret)
 	*is_new_ret = 0;
@@ -270,6 +281,20 @@ notmuch_config_open (void *ctx,
 	}
     }
 
+    /* If we have no configuration file then we configure "inbox" and "unread"
+     * tags by default for 'notmuch new'. This ensures that the Emacs mode
+     * would still work as expected.
+     *
+     * We do not ask the user for tags to be used by 'notmuch new'. That's too
+     * much detail for beginners and others can edit the configuration file by
+     * hand.
+     */
+    if (is_new) {
+	notmuch_config_set_new_tags (config, def_new_tags,
+				     sizeof(def_new_tags) /
+				     sizeof(const char*));
+    }
+
     /* When we create a new configuration file here, we  add some
      * comments to help the user understand what can be done. */
     if (is_new) {
@@ -279,6 +304,8 @@ notmuch_config_open (void *ctx,
 				database_config_comment, NULL);
 	g_key_file_set_comment (config->key_file, "user", NULL,
 				user_config_comment, NULL);
+	g_key_file_set_comment (config->key_file, "new", NULL,
+				new_config_comment, NULL);
     }
 
     if (is_new_ret)
@@ -494,3 +521,16 @@ notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)
     *length = config->new_tags_length;
     return config->new_tags;
 }
+
+void
+notmuch_config_set_new_tags (notmuch_config_t *config,
+			     const char *tags[],
+			     size_t length)
+{
+    g_key_file_set_string_list (config->key_file,
+				"new", "tags",
+				tags, length);
+
+    talloc_free (config->new_tags);
+    config->user_other_email = NULL;
+}
diff --git a/notmuch-setup.c b/notmuch-setup.c
index d06fbf8..c1406db 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -96,6 +96,8 @@ notmuch_setup_command (unused (void *ctx),
     notmuch_config_t *config;
     char **old_other_emails;
     size_t old_other_emails_len;
+    char **new_tags;
+    unsigned int new_tags_len;
     GPtrArray *other_emails;
     unsigned int i;
     int is_new;
@@ -147,6 +149,18 @@ notmuch_setup_command (unused (void *ctx),
 					     other_emails->len);
     g_ptr_array_free (other_emails, TRUE);
 
+    /* If we already have a configuration file then we preserve the tags
+     * configured there. If the original configuration file contains no tags
+     * then we assume that the user configured it that way and add no tags.
+     */
+    if (!is_new) {
+	new_tags = notmuch_config_get_new_tags (config, &new_tags_len);
+	if (new_tags) {
+	    notmuch_config_set_new_tags (config, (const char**)new_tags,
+					 new_tags_len);
+	}
+    }
+
     prompt ("Top-level directory of your email archive [%s]: ",
 	    notmuch_config_get_database_path (config));
     if (strlen (response)) {
-- 
1.6.3.3

[PATCH 4/4] notmuch-new: New cmdline option --tag=<name>.

[Jan Janak]

The list of tags to be applied by 'notmuch new' can be configured in
the configuration file. This command line option can be used to
override the list of tags from the coonfiguration file on the command
line. You may repeat the option several times if you want to apply
more than one tag:

  notmuch new --tag=apple --tag=orange

This is useful, for example, if you have an archive of messages you
would like to add to the database with a special tag so that they can
be easily identified later. To do that, you could simply copy the files
from the archive to the database directory and then index them all with:

  notmuch new --tag=prehistory

Tags to be applied every time 'notmuch new' is run can be specified in
the configuration file. One-time tags for individual runs can be
specified on the command line with this new option.

Signed-off-by: Jan Janak <jan@ryngle.com>
---
 notmuch-new.c |   40 ++++++++++++++++++++++++++++++++++------
 notmuch.c     |    8 +++++++-
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 10745e8..94036da 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -26,6 +26,9 @@ static volatile sig_atomic_t do_add_files_print_progress = 0;
 
 static notmuch_config_t *config = NULL;
 
+static char **cmdline_tags = NULL;
+static unsigned int cmdline_tags_count = 0;
+
 static void
 handle_sigalrm (unused (int signal))
 {
@@ -76,12 +79,19 @@ apply_tags (notmuch_message_t *message)
     char** tags;
     unsigned int count, i;
 
-    if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
-	return;
+    if (cmdline_tags_count) {
+	for (i = 0; i < cmdline_tags_count; i++) {
+	    if (cmdline_tags[i])
+		notmuch_message_add_tag (message, cmdline_tags[i]);
+	}
+    } else {
+	if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
+	    return;
 
-    for (i = 0; i < count; i++) {
-	if (tags[i])
-	    notmuch_message_add_tag (message, tags[i]);
+	for (i = 0; i < count; i++) {
+	    if (tags[i])
+		notmuch_message_add_tag (message, tags[i]);
+	}
     }
 }
 
@@ -413,7 +423,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     int ret = 0;
     struct stat st;
     const char *db_path;
-    char *dot_notmuch_path;
+    char *dot_notmuch_path, *opt;
+    char **tmp;
     struct sigaction action;
     int i;
 
@@ -423,6 +434,23 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
 	    add_files_state.verbose = 1;
+	} else if (STRNCMP_LITERAL (argv[i], "--tag=") == 0) {
+	    opt = argv[i] + sizeof ("--tag=") - 1;
+	    /* FIXME: We should check for leading and trailing white-space in
+	     * option value here and remove it.
+	     */
+	    if (*opt == '\0') {
+		fprintf (stderr, "Option value missing: %s\n", argv[i]);
+		return 1;
+	    }
+	    tmp = talloc_realloc (ctx, cmdline_tags, char*,
+				  cmdline_tags_count + 1);
+	    if (tmp == NULL) {
+		fprintf (stderr, "Notmuch ran out of memory.\n");
+		return 1;
+	    }
+	    tmp[cmdline_tags_count++] = opt;
+	    cmdline_tags = tmp;
 	} else {
 	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
 	    return 1;
diff --git a/notmuch.c b/notmuch.c
index b84e284..fb0c2a7 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -123,7 +123,7 @@ command_t commands[] = {
       "\t\tInvoking notmuch with no command argument will run setup if\n"
       "\t\tthe setup command has not previously been completed." },
     { "new", notmuch_new_command,
-      "[--verbose]",
+      "[--verbose] [--tag=<name>]",
       "\t\tFind and import new messages to the notmuch database.",
       "\t\tScans all sub-directories of the mail directory, performing\n"
       "\t\tfull-text indexing on new messages that are found. Each new\n"
@@ -145,6 +145,12 @@ command_t commands[] = {
       "\t\t\tVerbose operation. Shows paths of message files as\n"
       "\t\t\tthey are being indexed.\n"
       "\n"
+      "\t\t--tag=<name>\n"
+      "\n"
+      "\t\t\tAdd the tag <name> to all messages newly added to the\n"
+      "\t\t\tdatabase. You may repeat this option several times if\n"
+      "\t\t\tyou want to add more tags.\n"
+      "\n"
       "\t\tNote: \"notmuch new\" runs (other than the first run) will\n"
       "\t\tskip any read-only directories, so you can use that to mark\n"
       "\t\tdirectories that will not receive any new mail (and make\n"
-- 
1.6.3.3

Re: [PATCH 0/4] Make tags applied by 'notmuch new' configurable.

[Brett Viren]

On Tue, Nov 24, 2009 at 5:10 PM, Jan Janak <jan@ryngle.com> wrote:
> I would like to propose that we make the list of tags applied by 'notmuch new'
> configurable. Right now notmuch applies two tags to all new messages added to
> the database, 'inbox' and 'unread'. The two tags are added by the C code in
> notmuch-new.c and they cannot be changed without editing the source file and
> recompiling notmuch.

Personally I would like this.  I can't find myself leaving GNUS and so
am using notmuch as "just" a search engine.  Now I manually remove
"inbox" and "unread" after each "new" since they just clutter up an
inbox I never read.

Cheers,
-Brett.

Re: [PATCH 0/4] Make tags applied by 'notmuch new' configurable.

[Bdale Garbee]

On Tue, 2009-11-24 at 23:10 +0100, Jan Janak wrote:
> Instead of 'inbox' and 'unread', I configure
> 'notmuch new' to add a new tag called 'new' (and only that one). This tag
> marks newly added messages that haven't been properly tagged yet by my 
> auto-tagging scripts. 

Oh, brilliant!  I like it.

Bdale

Re: [PATCH 0/4] Make tags applied by 'notmuch new' configurable.

[Bart Trojanowski]

* Jan Janak <jan@ryngle.com> [091124 17:11]:
> I would like to propose that we make the list of tags applied by 'notmuch new'
> configurable. Right now notmuch applies two tags to all new messages added to
> the database, 'inbox' and 'unread'. The two tags are added by the C code in
> notmuch-new.c and they cannot be changed without editing the source file and
> recompiling notmuch.

I like it.

-Bart

-- 
				WebSig: http://www.jukie.net/~bart/sig/

Re: [PATCH 4/4] notmuch-new: New cmdline option --tag=<name>.

[Karl Wiberg]

On Tue, Nov 24, 2009 at 11:10 PM, Jan Janak <jan@ryngle.com> wrote:
> The list of tags to be applied by 'notmuch new' can be configured in
> the configuration file. This command line option can be used to
> override the list of tags from the coonfiguration file on the command
> line. You may repeat the option several times if you want to apply
> more than one tag:
>
>  notmuch new --tag=apple --tag=orange

This leaves no way to skip the tags in the config file without adding
at least one tag on the command line, right? It might be more flexible
to e.g. have the commandline tags applied in _addition_ to the tags in
the config, and have a --no-config-tags flag to skip the tags in the
config when desired. Or, if that seems more convenient, have a
--config-tags=yes|no flag that defaults to yes when no --tag is given,
and to no when at least one --tag is given.

-- 
Karl Wiberg, kha@treskal.com
   subrabbit.wordpress.com
   www.treskal.com/kalle

Re: [PATCH 4/4] notmuch-new: New cmdline option --tag=<name>.

[Jan Janak]

Hi Karl,

On Wed, Nov 25, 2009 at 7:21 AM, Karl Wiberg <kha@treskal.com> wrote:
> On Tue, Nov 24, 2009 at 11:10 PM, Jan Janak <jan@ryngle.com> wrote:
>> The list of tags to be applied by 'notmuch new' can be configured in
>> the configuration file. This command line option can be used to
>> override the list of tags from the coonfiguration file on the command
>> line. You may repeat the option several times if you want to apply
>> more than one tag:
>>
>>  notmuch new --tag=apple --tag=orange
>
> This leaves no way to skip the tags in the config file without adding
> at least one tag on the command line, right?

Right.

> It might be more flexible
> to e.g. have the commandline tags applied in _addition_ to the tags in
> the config, and have a --no-config-tags flag to skip the tags in the
> config when desired.

Yeah, I think that's a very good idea. I'll submit another patch for
that. Thanks for the suggestion!

  -- Jan

[PATCH] notmuch-new: Option to disable tags from the configuration file.

[Jan Janak]

This patch slightly changes behavior of the command line option --tag.
Any tags specified by the user on the command line will be added
*in addition* to tags configured in the configuration file.

This behavior can be changed with the new command line option
--no-config-tags. The user can use this option to ignore any tags from
the configuration file (i.e. only tags specified on the command line
will be applied).

With this new option the user can configure 'notmuch new' not to apply
any tags (and that was not possible before):

  notmuch new --no-config-tags

Suggested by Karl Wiberg.

Signed-off-by: Jan Janak <jan@ryngle.com>
---
 notmuch-new.c |   19 +++++++++++--------
 notmuch.c     |   11 ++++++++++-
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 94036da..e988bf1 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -28,6 +28,7 @@ static notmuch_config_t *config = NULL;
 
 static char **cmdline_tags = NULL;
 static unsigned int cmdline_tags_count = 0;
+static int add_config_tags = 1;
 
 static void
 handle_sigalrm (unused (int signal))
@@ -79,19 +80,19 @@ apply_tags (notmuch_message_t *message)
     char** tags;
     unsigned int count, i;
 
+    if (add_config_tags) {
+	tags = notmuch_config_get_new_tags (config, &count);
+	for (i = 0; tags && (i < count); i++) {
+	    if (tags[i])
+		notmuch_message_add_tag (message, tags[i]);
+	}
+    }
+
     if (cmdline_tags_count) {
 	for (i = 0; i < cmdline_tags_count; i++) {
 	    if (cmdline_tags[i])
 		notmuch_message_add_tag (message, cmdline_tags[i]);
 	}
-    } else {
-	if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
-	    return;
-
-	for (i = 0; i < count; i++) {
-	    if (tags[i])
-		notmuch_message_add_tag (message, tags[i]);
-	}
     }
 }
 
@@ -451,6 +452,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	    }
 	    tmp[cmdline_tags_count++] = opt;
 	    cmdline_tags = tmp;
+	} else if (STRNCMP_LITERAL (argv[i], "--no-config-tags") == 0) {
+	    add_config_tags = 0;
 	} else {
 	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
 	    return 1;
diff --git a/notmuch.c b/notmuch.c
index 1bd3265..ff8d5bb 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -149,7 +149,16 @@ command_t commands[] = {
       "\n"
       "\t\t\tAdd the tag <name> to all messages newly added to the\n"
       "\t\t\tdatabase. You may repeat this option several times if\n"
-      "\t\t\tyou want to add more tags.\n"
+      "\t\t\tyou want to add more tags. Any tags configured in the\n"
+      "\t\t\tconfiguration file will still be added, unless you also\n"
+      "\t\t\tuse the option --no-config-tags.\n"
+      "\n"
+      "\t\t--no-config-tags\n"
+      "\n"
+      "\t\t\tDo not add tags configured in the configuration file.\n"
+      "\t\t\tUse this option if you only want to add tags specified\n"
+      "\t\t\ton the command line with --tag or if do not want to add\n"
+      "\t\t\tany tags at all.\n"
       "\n"
       "\t\tNote: \"notmuch new\" runs (other than the first run) will\n"
       "\t\tskip any read-only directories, so you can use that to mark\n"
-- 
1.6.3.3

Re: [PATCH] notmuch-new: Option to disable tags from the configuration file.

[Bart Trojanowski]

Jan,

I think your use of STRNCMP_LITERAL here is wrong...

> +	} else if (STRNCMP_LITERAL (argv[i], "--no-config-tags") == 0) {
> +	    add_config_tags = 0;

it will happily match "--no-config-tags-xxxxxxxxxxxxxxxxxxx".

Can I also suggest including --no-config-tags in the 'notmuch help'
output?

Besides that I am very happy with this patch series.

-pBart

-- 
				WebSig: http://www.jukie.net/~bart/sig/

Re: [PATCH 2/4] notmuch: Config option to specify tags to be applied by 'notmuch new'.

[Bart Trojanowski]

Jan,

I really want this feature to get in, so I am going to do my best to
review your code :)

Here are some more sticking points...

> +char **
> +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);

If you are not giving over control of the pointer to the caller please
return const char * const *.

Similarly...

> +    char **new_tags;

... this should probably be const char **.

Next...

> +char **
> +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)

... but ...

> +    unsigned int count, i;
> +
> +    if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
> +	return;

size_t != unsigned int on all platforms.  Please stick with one or the
other.  Note there are a few calls to fix here.

-Bart

-- 
				WebSig: http://www.jukie.net/~bart/sig/

Re: [PATCH] notmuch-new: Option to disable tags from the configuration file.

[Jan Janak]

Hi Bart,

On 25-11 14:55, Bart Trojanowski wrote:
> Jan,
> 
> I think your use of STRNCMP_LITERAL here is wrong...
> 
> > +	} else if (STRNCMP_LITERAL (argv[i], "--no-config-tags") == 0) {
> > +	    add_config_tags = 0;
> 
> it will happily match "--no-config-tags-xxxxxxxxxxxxxxxxxxx".

Ahh, I actually stole the code from the part which parses the --verbose
command line option. And I didn't check it. That one suffers from the same
problem, so I'll just fix them both in one patch.

> Can I also suggest including --no-config-tags in the 'notmuch help'
> output?

Sure, I'll add it.

> Besides that I am very happy with this patch series.

Thanks a lot!

  -- Jan

Re: [PATCH 2/4] notmuch: Config option to specify tags to be applied by 'notmuch new'.

[Jan Janak]

Hi Bart,

On 25-11 15:56, Bart Trojanowski wrote:
> Jan,
> 
> I really want this feature to get in, so I am going to do my best to
> review your code :)
> 
> Here are some more sticking points...
> 
> > +char **
> > +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);
> 
> If you are not giving over control of the pointer to the caller please
> return const char * const *.

I followed Carl's style there, in particular the following function:
notmuch_config_get_user_other_email

I can, of course, change that. But maybe we should wait for Carl to see
which way he prefers.

> Similarly...
> 
> > +    char **new_tags;
> 
> ... this should probably be const char **.

That's the same story. I followed user_other_email there.

> Next...
> 
> > +char **
> > +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)
> 
> ... but ...
> 
> > +    unsigned int count, i;
> > +
> > +    if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
> > +	return;
> 
> size_t != unsigned int on all platforms.  Please stick with one or the
> other.  Note there are a few calls to fix here.

That's a good catch. I will fix that one. Thanks a lot for the review, I
really appreciate that!

  -- Jan

[PATCH 1/5] notmuch-new: Remove tag_add_inbox_unread in favor of a generic solution.

[Jan Janak]

Instead of adding 'inbox' and 'unread' tags directly in the code of
'notmuch-new', we can specify a list of tags to be added to newly
created messages with a configuration file option or a command line
option. That's more flexible, it allows the user to select which tags
should be added.

Signed-off-by: Jan Janak <jan@ryngle.com>
---
 notmuch-new.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index e32b92a..9970407 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -41,13 +41,6 @@ handle_sigint (unused (int sig))
 }
 
 static void
-tag_inbox_and_unread (notmuch_message_t *message)
-{
-    notmuch_message_add_tag (message, "inbox");
-    notmuch_message_add_tag (message, "unread");
-}
-
-static void
 add_files_print_progress (add_files_state_t *state)
 {
     struct timeval tv_now;
@@ -198,7 +191,6 @@ add_files_recursive (notmuch_database_t *notmuch,
 		    /* success */
 		    case NOTMUCH_STATUS_SUCCESS:
 			state->added_messages++;
-			tag_inbox_and_unread (message);
 			break;
 		    /* Non-fatal issues (go on to next file) */
 		    case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
-- 
1.6.3.3

[PATCH 2/5] notmuch: Config option to specify tags to be applied by 'notmuch new'.

[Jan Janak]

Add support for section [new] in the configuration file. This section
is supposed to contain options for 'notmuch new'. Currently there is
only one option called tags.

The tags option can be used to configure a set of tags to be applied
by 'notmuch new'. Individual tags are separated by semicolon.

'notmuch new' is modified not to apply 'inbox' and 'unread' by default,
but instead it obtains the set of tags to be applied from the new
configuration file option.

Signed-off-by: Jan Janak <jan@ryngle.com>

This revision of the patch includes suggestions from Bart Trojanowski.
---
 notmuch-client.h |    3 +++
 notmuch-config.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 notmuch-new.c    |   19 ++++++++++++++++++-
 3 files changed, 64 insertions(+), 1 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index c04eaeb..0fb9c19 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -184,6 +184,9 @@ notmuch_config_set_user_other_email (notmuch_config_t *config,
 				     const char *other_email[],
 				     size_t length);
 
+char **
+notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);
+
 notmuch_bool_t
 debugger_is_active (void);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index fc65d6b..57072ce 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -22,6 +22,7 @@
 
 #include <pwd.h>
 #include <netdb.h>
+#include <ctype.h>
 
 static const char toplevel_config_comment[] =
     " .notmuch-config - Configuration file for the notmuch mail system\n"
@@ -62,6 +63,9 @@ struct _notmuch_config {
     char *user_primary_email;
     char **user_other_email;
     size_t user_other_email_length;
+
+    char **new_tags;
+    size_t new_tags_length;
 };
 
 static int
@@ -199,6 +203,8 @@ notmuch_config_open (void *ctx,
     config->user_primary_email = NULL;
     config->user_other_email = NULL;
     config->user_other_email_length = 0;
+    config->new_tags = NULL;
+    config->new_tags_length = 0;
 
     if (! g_key_file_load_from_file (config->key_file,
 				     config->filename,
@@ -450,3 +456,40 @@ notmuch_config_set_user_other_email (notmuch_config_t *config,
     talloc_free (config->user_other_email);
     config->user_other_email = NULL;
 }
+
+char **
+notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)
+{
+    char **tags;
+    size_t i, len;
+    char *start, *end;
+
+    if (config->new_tags == NULL) {
+	config->new_tags_length = 0;
+	tags = g_key_file_get_string_list (config->key_file, "new", "tags",
+					   &len, NULL);
+
+	if (tags) {
+	    config->new_tags = talloc_size (config, sizeof(char*) *
+					    (len + 1));
+	    for (i = 0; i < len; i++) {
+		/* Remove leading and trailing white space around the tag and
+		 * filter out empty tags. */
+		start = tags[i];
+		end = start + strlen (start) - 1;
+		while (isspace (*start)) start++;
+		while (end > start && isspace (*end)) end--;
+		if (end >= start) {
+		    config->new_tags[config->new_tags_length++] =
+			talloc_strndup (config->new_tags, start,
+					end - start + 1);
+		}
+	    }
+	    config->new_tags[config->new_tags_length] = NULL;
+	    g_strfreev (tags);
+	}
+    }
+
+    *length = config->new_tags_length;
+    return config->new_tags;
+}
diff --git a/notmuch-new.c b/notmuch-new.c
index 9970407..af717b7 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -24,6 +24,8 @@
 
 static volatile sig_atomic_t do_add_files_print_progress = 0;
 
+static notmuch_config_t *config = NULL;
+
 static void
 handle_sigalrm (unused (int signal))
 {
@@ -68,6 +70,21 @@ add_files_print_progress (add_files_state_t *state)
     fflush (stdout);
 }
 
+static void
+apply_tags (notmuch_message_t *message)
+{
+    char** tags;
+    size_t count, i;
+
+    if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
+	return;
+
+    for (i = 0; i < count; i++) {
+	if (tags[i])
+	    notmuch_message_add_tag (message, tags[i]);
+    }
+}
+
 static int ino_cmp(const struct dirent **a, const struct dirent **b)
 {
     return ((*a)->d_ino < (*b)->d_ino) ? -1 : 1;
@@ -191,6 +208,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 		    /* success */
 		    case NOTMUCH_STATUS_SUCCESS:
 			state->added_messages++;
+			apply_tags (message);
 			break;
 		    /* Non-fatal issues (go on to next file) */
 		    case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
@@ -388,7 +406,6 @@ count_files (const char *path, int *count)
 int
 notmuch_new_command (void *ctx, int argc, char *argv[])
 {
-    notmuch_config_t *config;
     notmuch_database_t *notmuch;
     add_files_state_t add_files_state;
     double elapsed;
-- 
1.6.3.3

[PATCH 3/5] notmuch-setup: Copy/create the new section with tags for 'notmuch-new'.

[Jan Janak]

If the user runs 'notmuch setup' and there is no configuration file yet
then we also add the new section [new] to the configuration file and
set the tags option as:

  tags=inbox;unread

This will be picked up by 'notmuch new' and all new mail added to the
database will be tagged with the two tags as before.

If the user already has a configuration file and runs 'notmuch setup'
then we just copy whatever tags we find in the old configuration file
to the new one. If there are no tags in the old configuration file then
we assume that the user configured notmuch that way and the new config
file would also have no tags in the section [new].

We never ask the user interactively for the list of tags to be used by
'notmuch new', it is assumed that beginners would want to stick to the
defaults and advanced users can edit the configuration manually.

This revision of the patch includes suggestions from Bart Trojanowski.

Signed-off-by: Jan Janak <jan@ryngle.com>
---
 notmuch-client.h |    4 ++++
 notmuch-config.c |   40 ++++++++++++++++++++++++++++++++++++++++
 notmuch-setup.c  |   18 ++++++++++++++----
 3 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 0fb9c19..bb7d3d4 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -187,6 +187,10 @@ notmuch_config_set_user_other_email (notmuch_config_t *config,
 char **
 notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);
 
+void
+notmuch_config_set_new_tags (notmuch_config_t *config, const char *tags[],
+			     size_t length);
+
 notmuch_bool_t
 debugger_is_active (void);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index 57072ce..2bbeb20 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -54,6 +54,16 @@ static const char user_config_comment[] =
     " recipient list of replies, and will set the From address based on the\n"
     " address to which the original email was addressed.\n";
 
+static const char new_config_comment[] =
+    " Configuration section for 'notmuch new'\n"
+    "\n"
+    " The only supported value at the moment is 'tags. This option contains a\n"
+    " list of tags (separated by ';') that should be  automatically applied to\n"
+    " newly added messages.\n"
+    "\n"
+    " Note that 'notmuch new' also has a command line option which can be used\n"
+    " to add additional tags to the ones configured here.\n";
+
 struct _notmuch_config {
     char *filename;
     GKeyFile *key_file;
@@ -174,6 +184,7 @@ notmuch_config_open (void *ctx,
     GError *error = NULL;
     int is_new = 0;
     char *notmuch_config_env = NULL;
+    const char* def_new_tags[2] = {"inbox", "unread"};
 
     if (is_new_ret)
 	*is_new_ret = 0;
@@ -270,6 +281,20 @@ notmuch_config_open (void *ctx,
 	}
     }
 
+    /* If we have no configuration file then we configure "inbox" and "unread"
+     * tags by default for 'notmuch new'. This ensures that the Emacs mode
+     * would still work as expected.
+     *
+     * We do not ask the user for tags to be used by 'notmuch new'. That's too
+     * much detail for beginners and others can edit the configuration file by
+     * hand.
+     */
+    if (is_new) {
+	notmuch_config_set_new_tags (config, def_new_tags,
+				     sizeof(def_new_tags) /
+				     sizeof(const char*));
+    }
+
     /* When we create a new configuration file here, we  add some
      * comments to help the user understand what can be done. */
     if (is_new) {
@@ -279,6 +304,8 @@ notmuch_config_open (void *ctx,
 				database_config_comment, NULL);
 	g_key_file_set_comment (config->key_file, "user", NULL,
 				user_config_comment, NULL);
+	g_key_file_set_comment (config->key_file, "new", NULL,
+				new_config_comment, NULL);
     }
 
     if (is_new_ret)
@@ -493,3 +520,16 @@ notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)
     *length = config->new_tags_length;
     return config->new_tags;
 }
+
+void
+notmuch_config_set_new_tags (notmuch_config_t *config,
+			     const char *tags[],
+			     size_t length)
+{
+    g_key_file_set_string_list (config->key_file,
+				"new", "tags",
+				tags, length);
+
+    talloc_free (config->new_tags);
+    config->user_other_email = NULL;
+}
diff --git a/notmuch-setup.c b/notmuch-setup.c
index d06fbf8..aeffb88 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -92,12 +92,10 @@ notmuch_setup_command (unused (void *ctx),
 		       unused (int argc), unused (char *argv[]))
 {
     char *response = NULL;
-    size_t response_size;
+    size_t response_size, old_other_emails_len, new_tags_len, i;
     notmuch_config_t *config;
-    char **old_other_emails;
-    size_t old_other_emails_len;
+    char **old_other_emails, **new_tags;
     GPtrArray *other_emails;
-    unsigned int i;
     int is_new;
 
 #define prompt(format, ...)				\
@@ -147,6 +145,18 @@ notmuch_setup_command (unused (void *ctx),
 					     other_emails->len);
     g_ptr_array_free (other_emails, TRUE);
 
+    /* If we already have a configuration file then we preserve the tags
+     * configured there. If the original configuration file contains no tags
+     * then we assume that the user configured it that way and add no tags.
+     */
+    if (!is_new) {
+	new_tags = notmuch_config_get_new_tags (config, &new_tags_len);
+	if (new_tags) {
+	    notmuch_config_set_new_tags (config, (const char**)new_tags,
+					 new_tags_len);
+	}
+    }
+
     prompt ("Top-level directory of your email archive [%s]: ",
 	    notmuch_config_get_database_path (config));
     if (strlen (response)) {
-- 
1.6.3.3

[PATCH 4/5] notmuch-new: New cmdline option --tag=<name>.

[Jan Janak]

The list of tags to be applied by 'notmuch new' can be configured in
the configuration file. This command line option can be used to
override the list of tags from the coonfiguration file on the command
line. You may repeat the option several times if you want to apply
more than one tag:

  notmuch new --tag=apple --tag=orange

This is useful, for example, if you have an archive of messages you
would like to add to the database with a special tag so that they can
be easily identified later. To do that, you could simply copy the files
from the archive to the database directory and then index them all with:

  notmuch new --tag=prehistory

Tags to be applied every time 'notmuch new' is run can be specified in
the configuration file. One-time tags for individual runs can be
specified on the command line with this new option.

This revision of the patch includes suggestions from Bart Trojanowski.

Signed-off-by: Jan Janak <jan@ryngle.com>
---
 notmuch-new.c |   40 ++++++++++++++++++++++++++++++++++------
 notmuch.c     |    8 +++++++-
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index af717b7..cfbc6aa 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -26,6 +26,9 @@ static volatile sig_atomic_t do_add_files_print_progress = 0;
 
 static notmuch_config_t *config = NULL;
 
+static char **cmdline_tags = NULL;
+static size_t cmdline_tags_count = 0;
+
 static void
 handle_sigalrm (unused (int signal))
 {
@@ -76,12 +79,19 @@ apply_tags (notmuch_message_t *message)
     char** tags;
     size_t count, i;
 
-    if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
-	return;
+    if (cmdline_tags_count) {
+	for (i = 0; i < cmdline_tags_count; i++) {
+	    if (cmdline_tags[i])
+		notmuch_message_add_tag (message, cmdline_tags[i]);
+	}
+    } else {
+	if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
+	    return;
 
-    for (i = 0; i < count; i++) {
-	if (tags[i])
-	    notmuch_message_add_tag (message, tags[i]);
+	for (i = 0; i < count; i++) {
+	    if (tags[i])
+		notmuch_message_add_tag (message, tags[i]);
+	}
     }
 }
 
@@ -413,7 +423,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     int ret = 0;
     struct stat st;
     const char *db_path;
-    char *dot_notmuch_path;
+    char *dot_notmuch_path, *opt;
+    char **tmp;
     struct sigaction action;
     int i;
 
@@ -423,6 +434,23 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
 	    add_files_state.verbose = 1;
+	} else if (STRNCMP_LITERAL (argv[i], "--tag=") == 0) {
+	    opt = argv[i] + sizeof ("--tag=") - 1;
+	    /* FIXME: We should check for leading and trailing white-space in
+	     * option value here and remove it.
+	     */
+	    if (*opt == '\0') {
+		fprintf (stderr, "Option value missing: %s\n", argv[i]);
+		return 1;
+	    }
+	    tmp = talloc_realloc (ctx, cmdline_tags, char*,
+				  cmdline_tags_count + 1);
+	    if (tmp == NULL) {
+		fprintf (stderr, "Notmuch ran out of memory.\n");
+		return 1;
+	    }
+	    tmp[cmdline_tags_count++] = opt;
+	    cmdline_tags = tmp;
 	} else {
 	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
 	    return 1;
diff --git a/notmuch.c b/notmuch.c
index f45b692..1bd3265 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -123,7 +123,7 @@ command_t commands[] = {
       "\t\tInvoking notmuch with no command argument will run setup if\n"
       "\t\tthe setup command has not previously been completed." },
     { "new", notmuch_new_command,
-      "[--verbose]",
+      "[--verbose] [--tag=<name>]",
       "\t\tFind and import new messages to the notmuch database.",
       "\t\tScans all sub-directories of the mail directory, performing\n"
       "\t\tfull-text indexing on new messages that are found. Each new\n"
@@ -145,6 +145,12 @@ command_t commands[] = {
       "\t\t\tVerbose operation. Shows paths of message files as\n"
       "\t\t\tthey are being indexed.\n"
       "\n"
+      "\t\t--tag=<name>\n"
+      "\n"
+      "\t\t\tAdd the tag <name> to all messages newly added to the\n"
+      "\t\t\tdatabase. You may repeat this option several times if\n"
+      "\t\t\tyou want to add more tags.\n"
+      "\n"
       "\t\tNote: \"notmuch new\" runs (other than the first run) will\n"
       "\t\tskip any read-only directories, so you can use that to mark\n"
       "\t\tdirectories that will not receive any new mail (and make\n"
-- 
1.6.3.3

[PATCH 5/5] notmuch-new: Option to disable tags from the configuration file.

[Jan Janak]

This patch slightly changes behavior of the command line option --tag.
Any tags specified by the user on the command line will be added
*in addition* to tags configured in the configuration file.

This behavior can be changed with the new command line option
--no-config-tags. The user can use this option to ignore any tags from
the configuration file (i.e. only tags specified on the command line
will be applied).

With this new option the user can configure 'notmuch new' not to apply
any tags (and that was not possible before):

  notmuch new --no-config-tags

Suggested by Karl Wiberg.

Bugfix: Fix parsing of --verbose cmdline option.

This is a revised version of the patch, incorporating suggestions made
by Bart Trojanowski. Thanks Bart!

Signed-off-by: Jan Janak <jan@ryngle.com>
---
 notmuch-new.c |   21 ++++++++++++---------
 notmuch.c     |   13 +++++++++++--
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index cfbc6aa..eaed701 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -28,6 +28,7 @@ static notmuch_config_t *config = NULL;
 
 static char **cmdline_tags = NULL;
 static size_t cmdline_tags_count = 0;
+static int add_config_tags = 1;
 
 static void
 handle_sigalrm (unused (int signal))
@@ -79,19 +80,19 @@ apply_tags (notmuch_message_t *message)
     char** tags;
     size_t count, i;
 
+    if (add_config_tags) {
+	tags = notmuch_config_get_new_tags (config, &count);
+	for (i = 0; tags && (i < count); i++) {
+	    if (tags[i])
+		notmuch_message_add_tag (message, tags[i]);
+	}
+    }
+
     if (cmdline_tags_count) {
 	for (i = 0; i < cmdline_tags_count; i++) {
 	    if (cmdline_tags[i])
 		notmuch_message_add_tag (message, cmdline_tags[i]);
 	}
-    } else {
-	if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
-	    return;
-
-	for (i = 0; i < count; i++) {
-	    if (tags[i])
-		notmuch_message_add_tag (message, tags[i]);
-	}
     }
 }
 
@@ -432,7 +433,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     add_files_state.output_is_a_tty = isatty (fileno (stdout));
 
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
-	if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
+	if (strcmp (argv[i], "--verbose") == 0) {
 	    add_files_state.verbose = 1;
 	} else if (STRNCMP_LITERAL (argv[i], "--tag=") == 0) {
 	    opt = argv[i] + sizeof ("--tag=") - 1;
@@ -451,6 +452,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	    }
 	    tmp[cmdline_tags_count++] = opt;
 	    cmdline_tags = tmp;
+	} else if (strcmp (argv[i], "--no-config-tags") == 0) {
+	    add_config_tags = 0;
 	} else {
 	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
 	    return 1;
diff --git a/notmuch.c b/notmuch.c
index 1bd3265..2bb38f3 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -123,7 +123,7 @@ command_t commands[] = {
       "\t\tInvoking notmuch with no command argument will run setup if\n"
       "\t\tthe setup command has not previously been completed." },
     { "new", notmuch_new_command,
-      "[--verbose] [--tag=<name>]",
+      "[--verbose] [--tag=<name>] [--no-config-tags]",
       "\t\tFind and import new messages to the notmuch database.",
       "\t\tScans all sub-directories of the mail directory, performing\n"
       "\t\tfull-text indexing on new messages that are found. Each new\n"
@@ -149,7 +149,16 @@ command_t commands[] = {
       "\n"
       "\t\t\tAdd the tag <name> to all messages newly added to the\n"
       "\t\t\tdatabase. You may repeat this option several times if\n"
-      "\t\t\tyou want to add more tags.\n"
+      "\t\t\tyou want to add more tags. Any tags configured in the\n"
+      "\t\t\tconfiguration file will still be added, unless you also\n"
+      "\t\t\tuse the option --no-config-tags.\n"
+      "\n"
+      "\t\t--no-config-tags\n"
+      "\n"
+      "\t\t\tDo not add tags configured in the configuration file.\n"
+      "\t\t\tUse this option if you only want to add tags specified\n"
+      "\t\t\ton the command line with --tag or if do not want to add\n"
+      "\t\t\tany tags at all.\n"
       "\n"
       "\t\tNote: \"notmuch new\" runs (other than the first run) will\n"
       "\t\tskip any read-only directories, so you can use that to mark\n"
-- 
1.6.3.3

Re: [PATCH 0/4] Make tags applied by 'notmuch new' configurable.

[Jan Janak]

Hello,

On 24-11 23:10, Jan Janak wrote:
> I would like to propose that we make the list of tags applied by 'notmuch new'
> configurable. Right now notmuch applies two tags to all new messages added to
> the database, 'inbox' and 'unread'. The two tags are added by the C code in
> notmuch-new.c and they cannot be changed without editing the source file and
> recompiling notmuch.
> 
> The four patches that follow this email allow for configuring the tags to be
> added by 'notmuch new' either in the configuration file or on the command
> line.
> 
> This change was motivated by my desire to remove both tags from newly added
> messages. My rules for adding these two tags are more complex and I do it in
> a script run after 'notmuch new'. Instead of 'inbox' and 'unread', I configure
> 'notmuch new' to add a new tag called 'new' (and only that one). This tag
> marks newly added messages that haven't been properly tagged yet by my 
> auto-tagging scripts. The last script I run after 'notmuch new' removes that
> tag. My auto-tagging scripts process only messages with the tag 'new'.
> 
> On a side note; It may seem logical to add/omit the tag 'unread' directly in 
> 'notmuch new' based on the Maildir flags extracted from the filename of the
> message. I suggest that we don't do that in 'notmuch new'. A better way would
> be writing a small script or command that can be run *after* 'notmuch new'.
> We could then distribute it with notmuch (maybe with other small tagging
> scripts for common situations). 
> 
> I think Maildir flags should be processed after 'notmuch new' is because if
> there are multiple copies of a message with different flags, we may need to
> see all flags from all filenames to set corresponding tags properly and we may
> also need to take the directory part into consideration (i.e. the new mail is
> in 'new', not 'cur').
> 
> The list of tags to be applied by notmuch can be configured in the
> configuration file. There is a new section [new] which contains configuration
> options for 'notmuch new'. There is only one option called 'tags'. The option
> contains a semicolon separated list of tags:
> 
>   [new]
>   tags=inbox;unread  # Emulate the original behavior
> 
> One of the patches updates 'notmuch setup' to create the section and add
> the tags option with tags 'inbox' and 'unread', but only if a new
> configuration file is being created. If the configuration file already exists
> then it just copies the contents from the old configuration file to the new
> one.
> 
> We do not ask the user for the list of tags in the interactive part, that would
> have been too much. Users can edit the configuration file manually if they want
> to change the list of tags. If they create a new configuration file then they
> probably want to accept the default anyway.
> 
> There is one catch for users who already have a configuration file and start
> using the patches. They will need to add the new section and the tags option
> manually if they want to preserve current behavior of applying 'inbox' and
> 'unread' automatically by 'notmuch new'.
> 
> The last patch in the set adds a new command line option to 'notmuch new'.
> The name of the option is --tag and it can be used to override any tags
> configured in the configuration file. For example:
> 
>   notmuch new --tag=outbox --tag=read
> 
> adds the tags 'outbox' and 'read' and ignores any tags from the configuration
> file.
> 
> Comments and opinions are welcome!

I updated this patch series, adding features and bug fixes suggested by others
on the list.

The command line option --tag now *adds* tags, so the tags configured in the
configuration file are still applied...unless you use the new option
--no-config-tag. With this option only the tags specified on the command line
will be applied by 'notmuch-new'.

  -- Jan

Re: [PATCH 0/4] Make tags applied by 'notmuch new' configurable.

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 7962 bytes --]

On Tue, 24 Nov 2009 23:10:26 +0100, Jan Janak <jan@ryngle.com> wrote:
> I would like to propose that we make the list of tags applied by 'notmuch new'
> configurable. Right now notmuch applies two tags to all new messages added to
> the database, 'inbox' and 'unread'. The two tags are added by the C code in
> notmuch-new.c and they cannot be changed without editing the source file and
> recompiling notmuch.

Hi Jan,

Sorry to leave this thread sitting without review for so long. My travel
schedule, combined with a big US holiday last week combined to create a
fair amount of backlog.

> This change was motivated by my desire to remove both tags from newly added
> messages. My rules for adding these two tags are more complex and I do it in
> a script run after 'notmuch new'. Instead of 'inbox' and 'unread', I configure
> 'notmuch new' to add a new tag called 'new' (and only that one). This tag
> marks newly added messages that haven't been properly tagged yet by my 
> auto-tagging scripts. The last script I run after 'notmuch new' removes that
> tag. My auto-tagging scripts process only messages with the tag 'new'.

As a side-note, I would recommend against making your auto-tagging
scripts process only new messages. You can get a much more reliable
setup by having your auto-tagging scripts apply to the global
database. And this is not inefficient at all if you simply ensure that
you only match messages which need the tag added.

For example, in your current setup you presumably have something like:

	notmuch tag +foo <foo-specific-search-terms> and tag:new

If you change that to:

	notmuch tag +foo <foo-specific-search-terms> and not tag:foo

then you have the advantage of being able to change the search terms you
are using for the "foo" tag and know that this tag will be applied
globally, (and not only to messages incorporated since the rule
change). Also, you should find that this performs just fine.

You could also have a second specification that would support removing
the "foo" tag if the terms ever changed to become more restrictive:

	notmuch tag -foo tag:foo and not (<foo-specific-search-terms)

I've talked about doing "virtual tags" in the configuration file which
would implement basically the logic of these two "notmuch tag" commands
and be configured by simply providing the pair of:

	"foo" and <foo-specific-search-terms>

So, that's a slightly-off-topic recommendation for doing global tagging
rather than tagging based on new messages only. (Sup's support for
automatic tagging is only for new messages, and I learned there how
painful it can be to operate like that.)

OK, that was a rather long side-note.

Back to the patch, I do recognize that having "inbox" and "unread"
hard-coded within "notmuch new" is rather strict. So I'm definitely
interested in allowing something more flexible here. And I think the
configuration syntax you came up with makes a lot of sense for that.

> On a side note; It may seem logical to add/omit the tag 'unread' directly in 
> 'notmuch new' based on the Maildir flags extracted from the filename of the
> message. I suggest that we don't do that in 'notmuch new'. A better way would
> be writing a small script or command that can be run *after* 'notmuch new'.
> We could then distribute it with notmuch (maybe with other small tagging
> scripts for common situations). 

I do appreciate the sentiment here, (provide good support for
scriptability and let the user use that for extended functionality). But
the question is, how far do we take that idea? For example, we could
simply hard-code a single "new" tag and say that anyone that wants
"inbox" and "unread" on new messages should do:

	notmuch tag +inbox +unread -new tag:new

Would that make notmuch a more elegant system, or would it just be
harder to use than just allowing the inbox and unread tags in the
configuration file?

> I think Maildir flags should be processed after 'notmuch new' is because if
> there are multiple copies of a message with different flags, we may need to
> see all flags from all filenames to set corresponding tags properly and we may
> also need to take the directory part into consideration (i.e. the new mail is
> in 'new', not 'cur').

I think we're going to have to solve all of these problems anyway. We've
already got outstanding patches for dealing with message renames. And
the message rename case is identical to the case of duplicate messages
with different tags, (differing only in whether both filenames are still
present in the mail store).

> There is one catch for users who already have a configuration file and start
> using the patches. They will need to add the new section and the tags option
> manually if they want to preserve current behavior of applying 'inbox' and
> 'unread' automatically by 'notmuch new'.

Why not support backwards compatibility by simply treating an empty
configuration value as "inbox;unread"? We could even make the
notmuch-config code notice a case like this and write out the new
configuration file with the new option explicitly in it, (with
documentation).

I'd definitely like to see a smooth upgrade path where existing users
get as much benefit from a complete and well-documented configuration
file as new users. (I've been frustrated by too many programs where the
only way to get a well-documented configuration is to start from
scratch.)

> The last patch in the set adds a new command line option to 'notmuch new'.
> The name of the option is --tag and it can be used to override any tags
> configured in the configuration file. For example:
> 
>   notmuch new --tag=outbox --tag=read
> 
> adds the tags 'outbox' and 'read' and ignores any tags from the configuration
> file.

I don't like the idea of adding command-line-based tags to "notmuch
new", at least the way "notmuch new" currently works, (searching through
a mail store for any new files).

In the actual patch, you identified a use-case for this that is
important, (importing a new collection of messages and wanting them all
tagged together). The problem of doing this with the existing "notmuch
new" is that it's too easy to accidentally tag more than is
intended. The only way to make it work would be to:

	1. Turn off anything delivering mail to the mailstore

	2. Ensure that the delivery is actually complete somehow, (and
	   not still pending in local MTA queues, etc.)

	3. Run "notmuch new" to ensure all recently-delivered mail is
	   incorporated.

	4. Copy the new collection of mail into the mail store.

	5. Run "notmuch new --tag" with the collection-specific tag.

	6. Turn on mail delivery again.

That's too hard, and far too error-prone. (I don't know of any good way
to do step (2) on my system, for example.)

If we had something like "notmuch import" that took files (perhaps even
an mbox) and copied them into the mail store, then *that* command could
legitimately accept a --tag option.

Meanwhile, the way I think I'd like to see this operation supported is
by a search specification that operates on the filenames. People have
already asked for support for doing tagging based on directory names,
(which will apparently allow people to keep their collection of
Gmail-created tags). So I'd like to see something like this instead for
the use case:

	mv some-archive ~/mail-store/some-archive
        notmuch tag +some-archive filename:some-archive*

This idea seems very much in line with what you advocated earlier,
(handling the maildir "unread" flag after the fact, rather than in
"notmuch new").

And yes, I realize that I'm arguing on different sides of the issue
here. Trying to decide how much "notmuch new" should or shouldn't do is
a tricky thing, and I'm hoping that by arguing from both sides I'll
better be able to see the right answer. :-)

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/4] notmuch: Config option to specify tags to be applied by 'notmuch new'.

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]

On Wed, 25 Nov 2009 22:50:08 +0100, Jan Janak <jan@ryngle.com> wrote:
> On 25-11 15:56, Bart Trojanowski wrote:
> > I really want this feature to get in, so I am going to do my best to
> > review your code :)
> > 
> > Here are some more sticking points...
> > 
> > > +char **
> > > +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);
> > 
> > If you are not giving over control of the pointer to the caller please
> > return const char * const *.
> 
> I followed Carl's style there, in particular the following function:
> notmuch_config_get_user_other_email
>
> I can, of course, change that. But maybe we should wait for Carl to see
> which way he prefers.

Call me stupid. Anything more complex than "const char *" and I can
never figure out where to put the const to express what's desired.

If "const char * const *" means what we want it to, then feel free to
return that, (and fix up existing cases). But maybe add a little block
of example code to the documentation of the function so that idiots like
me can still figure out how to declare the right kind of variable to
call the function.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 0/4] Make tags applied by 'notmuch new' configurable.

[Marten Veldthuis]

Excerpts from Carl Worth's message of Wed Dec 02 22:36:13 +0100 2009:
> As a side-note, I would recommend against making your auto-tagging
> scripts process only new messages. You can get a much more reliable
> setup by having your auto-tagging scripts apply to the global
> database. And this is not inefficient at all if you simply ensure that
> you only match messages which need the tag added.

I can see one clear case where that would become a problem: mistaggings.
If I set up something so that in 99% of the cases, it'd tag new mail
correctly with say "inbox unread mytag", I could quickly pick the
mistagged ones out and remove the offending tag. 

If you do it like above however, the autotag script would come merrily
along and retag them.
-- 
- Marten

Re: [PATCH 0/4] Make tags applied by 'notmuch new' configurable.

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 2624 bytes --]

On Thu, 03 Dec 2009 10:48:00 +0100, Marten Veldthuis <marten@veldthuis.com> wrote:
> I can see one clear case where that would become a problem: mistaggings.
> If I set up something so that in 99% of the cases, it'd tag new mail
> correctly with say "inbox unread mytag", I could quickly pick the
> mistagged ones out and remove the offending tag. 
> 
> If you do it like above however, the autotag script would come merrily
> along and retag them.

Right. A fully automatic, rule-based tag does not allow for manual
deletion of the exact same tag, (nor even manual addition).

I've given this some thought and there are things we could do here. For
example, the automatic tag could be stored internally under a different
prefix than the manual tag of the same name. That would be enough to
support the manual addition of a tag, such that a user-level search such
as:

	tag:foo

would map internally to something like:

	(auto-tag:foo OR manual-tag:foo)

Then manual tag removal could similarly be stored as yet anther internal
prefix that would work in a negative sense. So the search for "tag:foo"
would now map to:

	((auto-tag:foo OR manual-tag:foo) AND NOT manual-tag-removal:foo) [*]

I'm not sure yet if this idea is simply clever or insanely stupid.

It does seem like we could do this and hide all of the complexity
entirely from the user. But hidden complexity can raise its head in
nasty ways. Such as "I wrote this rule in my configuration file, why
isn't it working?" (in the face of a hidden manual-tag-removal).

Another idea would be to just keep the rule-based tags strictly as
configured, explicitly prevent the user from manually adding/removing
tags of the same name. And then the user could play games like the above
in saved searches to be able to simulate manual touchups of rule-based
tags, (without the automatic vs. manual state being hidden).

Yet another idea is to keep all rule-based tagging out of the
configuration, and let the user come up with whatever scheme they most
prefer. This could still allow for the configuration to hold saved
searches, (which could stand in for rule-based tags in a lot of cases
and avoid a lot of the complexity discussed here).

So those are some of my current thoughts on the issue. And that's why I
haven't put any auto tagging into the configuration file yet.

-Carl

[*] Or should that be:

	(manual-tag:foo OR (auto-tag:foo AND NOT manual-tag-removal:foo))

It probably wouldn't matter in practice as presumably the addition of
the manual-tag would remove any manual-tag-removal and vice-versa.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]