From: Mark Walters <markwalters1009@gmail.com>
To: Peter Wang <novalazy@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH 10/18] insert: parse command-line tag operations
In-Reply-To: <1343223767-9812-10-git-send-email-novalazy@gmail.com>
References: <1343223767-9812-1-git-send-email-novalazy@gmail.com>
	<1343223767-9812-10-git-send-email-novalazy@gmail.com>
User-Agent: Notmuch/0.14+81~g9730584 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Sun, 18 Nov 2012 17:05:42 +0000
Message-ID: <87r4nqq1sp.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


Hi

On Wed, 25 Jul 2012, Peter Wang <novalazy@gmail.com> wrote:
> Parse +tag and -tag on the 'insert' command-line.
> Issue a warning about ambiguous -tag arguments which don't follow
> +tag nor an explicit option list terminator.
> ---
>  notmuch-insert.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 50 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 4fb3ea3..6db03e3 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -24,6 +24,11 @@
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  
> +typedef struct {
> +    const char *tag;
> +    notmuch_bool_t remove;
> +} tag_operation_t;
> +
>  static notmuch_bool_t
>  check_folder_name (const char *folder)
>  {
> @@ -236,8 +241,11 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
>      const char **new_tags;
>      size_t new_tags_length;
>      const char *folder = NULL;
> +    tag_operation_t *tag_ops;
> +    int tag_ops_count = 0;
>      char *maildir;
>      int opt_index;
> +    notmuch_bool_t warn_tag_rem;
>      notmuch_bool_t ret;
>  
>      notmuch_opt_desc_t options[] = {
> @@ -253,6 +261,48 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
>  	return 1;
>      }
>  
> +    if (opt_index > 0 && strcmp (argv[opt_index - 1], "--") == 0) {
> +	warn_tag_rem = FALSE;
> +    } else {
> +	warn_tag_rem = TRUE;
> +    }
> +
> +    /* Array of tagging operations (add or remove), terminated with an
> +     * empty element. */
> +    tag_ops = talloc_array (ctx, tag_operation_t, argc - opt_index + 1);
> +    if (tag_ops == NULL) {
> +	fprintf (stderr, "Out of memory.\n");
> +	return 1;
> +    }
> +
> +    for (; opt_index < argc; opt_index++) {
> +	if (argv[opt_index][0] == '+') {
> +	    tag_ops[tag_ops_count].tag = argv[opt_index] + 1;
> +	    tag_ops[tag_ops_count].remove = FALSE;
> +	    tag_ops_count++;
> +	    warn_tag_rem = FALSE;
> +	} else if (argv[opt_index][0] == '-') {
> +	    if (warn_tag_rem) {
> +		fprintf (stderr,
> +			 "Warning: ambiguous argument treated as tag removal: %s\n",
> +			 argv[opt_index]);
> +	    }
> +	    tag_ops[tag_ops_count].tag = argv[opt_index] + 1;
> +	    tag_ops[tag_ops_count].remove = TRUE;
> +	    tag_ops_count++;
> +	} else {
> +	    break;
> +	}
> +    }

I don't like the code duplication between this and notmuch-tag.c. In
particular the two (now) differ on how they deal with malformed tags
etc. Would it be possible to unify them?

Best wishes

Mark

> +
> +    tag_ops[tag_ops_count].tag = NULL;
> +
> +    if (opt_index != argc) {
> +	fprintf (stderr, "Error: bad argument to notmuch insert: %s\n",
> +		 argv[opt_index]);
> +	return 1;
> +    }
> +
>      config = notmuch_config_open (ctx, NULL, NULL);
>      if (config == NULL)
>  	return 1;
> -- 
> 1.7.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch