[PATCH] Configurable tags for notmuch new

[PATCH] Configurable tags for notmuch new

[Ben Gamari]

Hey all,

I had some time today to finally put together a patch to make the tags used by
notmuch new configurable. I added a [messages] section to the .notmuch-config
to hold this new option, although this can be easily changed. Let me know what
you think! Thanks!

- Ben

[PATCH] notmuch-config: make new message tags configurable

[Ben Gamari]

Add a new_tags option in the [messages] section of the configuration
file to allow the user to specify which tags should be added to new
messages by notmuch new.
---
 notmuch-client.h |    8 ++++++
 notmuch-config.c |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch-new.c    |   14 +++++------
 3 files changed, 77 insertions(+), 8 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index c80b39c..1830584 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -171,6 +171,14 @@ 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);
+void
+notmuch_config_set_new_tags (notmuch_config_t *config,
+			     const char *new_tags[],
+			     size_t length);
+
 notmuch_bool_t
 debugger_is_active (void);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index 95430db..b492bc1 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -37,6 +37,12 @@ static const char database_config_comment[] =
     " Notmuch will store its database within a sub-directory of the path\n"
     " configured here named \".notmuch\".\n";
 
+static const char messages_config_comment[] =
+    " Messages configuration\n"
+    "\n"
+    " The only value supported here is 'new_tags' which lists the tags that\n"
+    " should be applied to new messages.\n";
+
 static const char user_config_comment[] =
     " User configuration\n"
     "\n"
@@ -62,6 +68,8 @@ 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
@@ -169,6 +177,7 @@ notmuch_config_open (void *ctx,
 {
     GError *error = NULL;
     int is_new = 0;
+    size_t tmp;
     char *notmuch_config_env = NULL;
 
     if (is_new_ret)
@@ -199,6 +208,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,
@@ -264,6 +275,11 @@ notmuch_config_open (void *ctx,
 	}
     }
 
+    if (notmuch_config_get_new_tags (config, &tmp) == NULL) {
+        const char *tags[] = { "unread", "inbox" };
+	notmuch_config_set_new_tags (config, tags, 2);
+    }
+
     /* When we create a new configuration file here, we  add some
      * comments to help the user understand what can be done. */
     if (is_new) {
@@ -271,6 +287,8 @@ notmuch_config_open (void *ctx,
 				toplevel_config_comment, NULL);
 	g_key_file_set_comment (config->key_file, "database", NULL,
 				database_config_comment, NULL);
+	g_key_file_set_comment (config->key_file, "messages", NULL,
+				messages_config_comment, NULL);
 	g_key_file_set_comment (config->key_file, "user", NULL,
 				user_config_comment, NULL);
     }
@@ -452,3 +470,48 @@ 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 tags_length;
+    unsigned int i;
+
+    if (config->new_tags == NULL) {
+	tags = g_key_file_get_string_list (config->key_file,
+					   "messages", "new_tags",
+					   &tags_length, NULL);
+	if (tags) {
+	    config->new_tags = talloc_size (config,
+					    sizeof (char *) *
+					    (tags_length + 1));
+	    for (i = 0; i < tags_length; i++)
+		config->new_tags[i] = talloc_strdup (config->new_tags,
+						     tags[i]);
+	    config->new_tags[i] = NULL;
+
+	    g_strfreev (tags);
+
+	    config->new_tags_length = tags_length;
+	}
+    }
+
+    *length = config->new_tags_length;
+    return config->new_tags;
+}
+
+void
+notmuch_config_set_new_tags (notmuch_config_t *config,
+			     const char *new_tags[],
+			     size_t length)
+{
+    g_key_file_set_string_list (config->key_file,
+				"messages", "new_tags",
+				new_tags, length);
+
+    talloc_free (config->new_tags);
+    config->new_tags = NULL;
+}
+
diff --git a/notmuch-new.c b/notmuch-new.c
index 44b50aa..dd5a5a0 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -35,6 +35,8 @@ typedef struct _filename_list {
 typedef struct {
     int output_is_a_tty;
     int verbose;
+    char **new_tags;
+    size_t new_tags_length;
 
     int total_files;
     int processed_files;
@@ -94,13 +96,6 @@ _filename_list_add (_filename_list_t *list,
 }
 
 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;
@@ -222,6 +217,7 @@ add_files_recursive (notmuch_database_t *notmuch,
     notmuch_filenames_t *db_subdirs = NULL;
     struct stat st;
     notmuch_bool_t is_maildir, new_directory;
+    const char **tag;
 
     if (stat (path, &st)) {
 	fprintf (stderr, "Error reading directory %s: %s\n",
@@ -412,7 +408,8 @@ add_files_recursive (notmuch_database_t *notmuch,
 	/* success */
 	case NOTMUCH_STATUS_SUCCESS:
 	    state->added_messages++;
-	    tag_inbox_and_unread (message);
+	    for (tag=state->new_tags; *tag != NULL; tag++)
+	        notmuch_message_add_tag (message, *tag);
 	    break;
 	/* Non-fatal issues (go on to next file) */
 	case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
@@ -736,6 +733,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
+    add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length);
     db_path = notmuch_config_get_database_path (config);
 
     dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
-- 
1.7.0.2

Re: [PATCH] notmuch-config: make new message tags configurable

[Ben Gamari]

On Fri, 12 Mar 2010 17:13:26 -0500, Ben Gamari <bgamari.foss@gmail.com> wrote:
> Add a new_tags option in the [messages] section of the configuration
> file to allow the user to specify which tags should be added to new
> messages by notmuch new.

Would anyone oppose this being merged? I've been running with it on a daily
basis for the last two weeks and I've had no issues. The migration path is
quite clean as the current behavior is the default.

- Ben

Re: [PATCH] notmuch-config: make new message tags configurable

[Michal Sojka]

On Sun, 28 Mar 2010, Ben Gamari wrote:
> On Fri, 12 Mar 2010 17:13:26 -0500, Ben Gamari <bgamari.foss@gmail.com> wrote:
> > Add a new_tags option in the [messages] section of the configuration
> > file to allow the user to specify which tags should be added to new
> > messages by notmuch new.
> 
> Would anyone oppose this being merged? I've been running with it on a daily
> basis for the last two weeks and I've had no issues. The migration path is
> quite clean as the current behavior is the default.

Hi,

I think that your patch is only a temporary solution. Some people
requests the ability to tag messages based on the maildir flags so the
question is how would your patch integrate with maildir-based tagging?

Michal

Re: [PATCH] notmuch-config: make new message tags configurable

[Ben Gamari]

On Sun, 28 Mar 2010 07:57:30 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> On Sun, 28 Mar 2010, Ben Gamari wrote:
> > On Fri, 12 Mar 2010 17:13:26 -0500, Ben Gamari <bgamari.foss@gmail.com> wrote:
> > > Add a new_tags option in the [messages] section of the configuration
> > > file to allow the user to specify which tags should be added to new
> > > messages by notmuch new.
> > 
> > Would anyone oppose this being merged? I've been running with it on a daily
> > basis for the last two weeks and I've had no issues. The migration path is
> > quite clean as the current behavior is the default.
> 
> Hi,
> 
> I think that your patch is only a temporary solution. Some people
> requests the ability to tag messages based on the maildir flags so the
> question is how would your patch integrate with maildir-based tagging?
> 
I would argue that maildir-tagging and basic notmuch new tagging could be
orthogonal. I think maildir flag tagging should be handled completely by the
maildir mail store backend. I could be wrong though.

In fact, now that I think of it, perhaps this patch's functionality belongs in
the mail store backend as well. I haven't tried your patchset, but it looks
like it would be trivial to add this as a mailstore option.

- Ben

Re: [PATCH] notmuch-config: make new message tags configurable

[Michal Sojka]

On Sun, 28 Mar 2010, Ben Gamari wrote:
> On Sun, 28 Mar 2010 07:57:30 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrote:
> > On Sun, 28 Mar 2010, Ben Gamari wrote:
> > > On Fri, 12 Mar 2010 17:13:26 -0500, Ben Gamari <bgamari.foss@gmail.com> wrote:
> > > > Add a new_tags option in the [messages] section of the configuration
> > > > file to allow the user to specify which tags should be added to new
> > > > messages by notmuch new.
> > > 
> > > Would anyone oppose this being merged? I've been running with it on a daily
> > > basis for the last two weeks and I've had no issues. The migration path is
> > > quite clean as the current behavior is the default.
> > 
> > Hi,
> > 
> > I think that your patch is only a temporary solution. Some people
> > requests the ability to tag messages based on the maildir flags so the
> > question is how would your patch integrate with maildir-based tagging?
> > 
> I would argue that maildir-tagging and basic notmuch new tagging could be
> orthogonal. I think maildir flag tagging should be handled completely by the
> maildir mail store backend. I could be wrong though.
> 
> In fact, now that I think of it, perhaps this patch's functionality belongs in
> the mail store backend as well. I haven't tried your patchset, but it looks
> like it would be trivial to add this as a mailstore option.

I agree. I'll try to add your patch to my mailstore patch set. It will
be enabled only when the default mailstore (called files) is enabled.
I'll move the config option from [messages] to [mailstore] section and
rename it from new_tags to files.new_tags, to stress the fact, that it
applies only for files backed.

Do you agree?

Michal

Re: [PATCH] notmuch-config: make new message tags configurable

[Sebastian Spaeth]

On Fri, 12 Mar 2010 17:13:26 -0500, Ben Gamari <bgamari.foss@gmail.com> wrote:
>  	/* success */
>  	case NOTMUCH_STATUS_SUCCESS:
>  	    state->added_messages++;
> -	    tag_inbox_and_unread (message);
> +	    for (tag=state->new_tags; *tag != NULL; tag++)
> +	        notmuch_message_add_tag (message, *tag);

notmuch-new.c: In function 'add_files_recursive':
notmuch-new.c:465: warning: assignment from incompatible pointer typ

I was just trying to apply this (in addition to maildir based tagging)
to my branch, and I am getting a compiler warning. Any clue as to how to
fix this?

This is the exact commit:
http://github.com/spaetz/notmuch-all-feature/commit/9beead362971af818697412e9686f96078cdf925#L2R469

Sebastian

Re: [PATCH] notmuch-config: make new message tags configurable

[Carl Worth]

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

On Fri, 12 Mar 2010 17:13:26 -0500, Ben Gamari <bgamari.foss@gmail.com> wrote:
> Add a new_tags option in the [messages] section of the configuration
> file to allow the user to specify which tags should be added to new
> messages by notmuch new.

Thanks, Ben!

I finally broke down and decided I needed this feature.

Previously I've done all of my automatic tagging as global searches,
(and it's those tags that I want to switch to just search macros as I've
talked about many times in the past).

But I'm now doing things like a "merge window" tag where I want it
applied to new messages as they come in, but I also want to be able to
manually remove the tag from messages where it doesn't make sense. So a
global search doesn't do what I want here at all.

For me, I'll probably go the route of having "notmuch new" add a "new"
tag in addition to "inbox" and "unread" and then I'll have my
notmuch-poll script act on that, then remove the "new" tag from all
tagged messages. I wouldn't want to impose a "new" tag on everyone since
most people won't have a notmuch-poll removing "new" by default so that
tag would just build up.

Meanwhile, I know that Keith Packard and Eric Anholt are already having
their notmuch-poll scripts remove the "unread" tag from all message,
since they don't find that tag useful at all. This new configuration
will help them as well.

So I've now merged this patch, and as usual, I followed up with some
changes as well. Here's what I did in addition:

  * Fixed the compiler warning.

  * Fixed "notmuch setup" to add the documentation block to the
    configuration file when adding new options there.

  * Renamed the new configuration option from:

	[messages]
	new_tags=inbox;unread;

    to instead be:

	[new]
	tags=inbox;unread;

  * Fixed "notmuch setup" to prompt for this new setting.

Thanks again,

-Carl

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