Date: Thu, 19 Jan 2012 23:19:02 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Pieter Praet <pieter@praet.org>
Subject: Re: [PATCH 4/4] setup: prompt user for search.exclude_tags value
Message-ID: <20120120041902.GV16740@mit.edu>
References: <1326586654-16840-3-git-send-email-amdragon@mit.edu>
	<1327000744-25463-1-git-send-email-pieter@praet.org>
	<1327000744-25463-5-git-send-email-pieter@praet.org>
	<20120119194437.GP16740@mit.edu> <8762g7o0k4.fsf@praet.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8762g7o0k4.fsf@praet.org>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: Notmuch Mail <notmuch@notmuchmail.org>
Precedence: list

Quoth Pieter Praet on Jan 19 at 10:16 pm:
> On Thu, 19 Jan 2012 14:44:37 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > Quoth Pieter Praet on Jan 19 at  8:19 pm:
> > > Allow users to customize the search.exclude_tags option during setup.
> > > 
> > > ---
> > >  notmuch-setup.c |   36 ++++++++++++++++++++++++++++++++++++
> > >  1 files changed, 36 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/notmuch-setup.c b/notmuch-setup.c
> > > index c3ea937..44d4aaa 100644
> > > --- a/notmuch-setup.c
> > > +++ b/notmuch-setup.c
> > > @@ -101,6 +101,8 @@ notmuch_setup_command (unused (void *ctx),
> > >      int is_new;
> > >      const char **new_tags;
> > >      size_t new_tags_len;
> > > +    const char **search_exclude_tags;
> > > +    size_t search_exclude_tags_len;
> > >  
> > >  #define prompt(format, ...)					\
> > >      do {							\
> > > @@ -195,6 +197,40 @@ notmuch_setup_command (unused (void *ctx),
> > >  	g_ptr_array_free (tags, TRUE);
> > >      }
> > >  
> > > +    search_exclude_tags = notmuch_config_get_search_exclude_tags (config, &search_exclude_tags_len);
> > > +
> > > +    printf ("Tags to exclude when searching messages (separated by spaces) [");
> > > +
> > > +    for (i = 0; i < search_exclude_tags_len; i++) {
> > > +	if (i != 0)
> > > +	    printf (" ");
> > > +	printf ("%s", search_exclude_tags[i]);
> > > +    }
> > > +
> > > +    prompt ("]: ");
> > > +
> > > +    if (strlen (response)) {
> > > +	GPtrArray *tags = g_ptr_array_new ();
> > > +	char *tag = response;
> > > +	char *space;
> > > +
> > > +	while (tag && *tag) {
> > > +	    space = strchr (tag, ' ');
> > > +	    if (space)
> > > +		g_ptr_array_add (tags, talloc_strndup (ctx, tag, space - tag));
> > > +	    else
> > > +		g_ptr_array_add (tags, talloc_strdup (ctx, tag));
> > > +	    tag = space;
> > > +	    while (tag && *tag == ' ')
> > > +		tag++;
> > > +	}
> > > +
> > > +	notmuch_config_set_search_exclude_tags (config, (const char **) tags->pdata,
> > > +				     tags->len);
> > > +
> > > +	g_ptr_array_free (tags, TRUE);
> > > +    }
> > > +
> > 
> > Holy code duplication.  Can we move most of this (at least the
> > response parsing part and maybe the prompt printing) into a function
> > and use it for both new tags and exclude tags?
> > 
> 
> Depends on who "we" is... :)  I would gladly do it if I could, but I
> think this uber1337 copy-paste coding job serves as *very* convincing
> proof that I'm pretty much in the dark (with a single wet match) when it
> comes to C :)

Hah, okay.  How about this as an initial minor refactoring of the code
for new.tags?

diff --git a/notmuch-setup.c b/notmuch-setup.c
index c3ea937..dcfa607 100644
--- a/notmuch-setup.c
+++ b/notmuch-setup.c
@@ -87,6 +87,38 @@ welcome_message_post_setup (void)
 "have sufficient storage space available now.\n\n");
 }
 
+static void
+print_tag_list (const char **tags, size_t tags_len)
+{
+    unsigned int i;
+    for (i = 0; i < tags_len; i++) {
+	if (i != 0)
+	    printf (" ");
+	printf ("%s", tags[i]);
+    }
+}
+
+static GPtrArray *
+parse_tag_list (void *ctx, char *response)
+{
+    GPtrArray *tags = g_ptr_array_new ();
+    char *tag = response;
+    char *space;
+
+    while (tag && *tag) {
+	space = strchr (tag, ' ');
+	if (space)
+	    g_ptr_array_add (tags, talloc_strndup (ctx, tag, space - tag));
+	else
+	    g_ptr_array_add (tags, talloc_strdup (ctx, tag));
+	tag = space;
+	while (tag && *tag == ' ')
+	    tag++;
+    }
+
+    return tags;
+}
+
 int
 notmuch_setup_command (unused (void *ctx),
 		       unused (int argc), unused (char *argv[]))
@@ -164,30 +196,11 @@ notmuch_setup_command (unused (void *ctx),
     new_tags = notmuch_config_get_new_tags (config, &new_tags_len);
 
     printf ("Tags to apply to all new messages (separated by spaces) [");
-
-    for (i = 0; i < new_tags_len; i++) {
-	if (i != 0)
-	    printf (" ");
-	printf ("%s", new_tags[i]);
-    }
-
+    print_tag_list(new_tags, new_tags_len);
     prompt ("]: ");
 
     if (strlen (response)) {
-	GPtrArray *tags = g_ptr_array_new ();
-	char *tag = response;
-	char *space;
-
-	while (tag && *tag) {
-	    space = strchr (tag, ' ');
-	    if (space)
-		g_ptr_array_add (tags, talloc_strndup (ctx, tag, space - tag));
-	    else
-		g_ptr_array_add (tags, talloc_strdup (ctx, tag));
-	    tag = space;
-	    while (tag && *tag == ' ')
-		tag++;
-	}
+	GPtrArray *tags = parse_tag_list (ctx, response);
 
 	notmuch_config_set_new_tags (config, (const char **) tags->pdata,
 				     tags->len);