From: Jani Nikula <jani@nikula.org>
To: Peter Wang <novalazy@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v3 07/20] tag-util: do not reset list in
	parse_tag_command_line
In-Reply-To: <1358643004-14522-8-git-send-email-novalazy@gmail.com>
References: <1358643004-14522-1-git-send-email-novalazy@gmail.com>
	<1358643004-14522-8-git-send-email-novalazy@gmail.com>
User-Agent: Notmuch/0.14+259~gdee88db (http://notmuchmail.org) Emacs/23.2.1
	(x86_64-pc-linux-gnu)
Date: Mon, 21 Jan 2013 19:09:04 +0100
Message-ID: <878v7m2ytb.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Sun, 20 Jan 2013, Peter Wang <novalazy@gmail.com> wrote:
> No current callers of parse_tag_command_line require that it clear its
> tag list argument.  The notmuch 'insert' command will be better served
> if the function modifies a pre-populated list (of new.tags) instead of
> clobbering it outright.

I think I'd like parse_tag_command_line() and parse_tag_line() to behave
similarly, both either resetting or not resetting tag list. I think you
could just parse command line first, and add new.tags afterwards, and
you'd be fine without changing parse_tag_command_line().

> ---
>  tag-util.c | 2 --
>  tag-util.h | 2 ++
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tag-util.c b/tag-util.c
> index 701d329..3f9da05 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -165,8 +165,6 @@ parse_tag_command_line (void *ctx, int argc, char **argv,
>  
>      int i;
>  
> -    tag_op_list_reset (tag_ops);
> -
>      for (i = 0; i < argc; i++) {
>  	if (strcmp (argv[i], "--") == 0) {
>  	    i++;
> diff --git a/tag-util.h b/tag-util.h
> index 246de85..4628f16 100644
> --- a/tag-util.h
> +++ b/tag-util.h
> @@ -81,6 +81,8 @@ parse_tag_line (void *ctx, char *line,
>   * Output Parameters:
>   *	ops	contains a list of tag operations
>   *	query_str the search terms.
> + *
> + * The ops argument is not cleared.
>   */
>  
>  tag_parse_status_t
> -- 
> 1.7.12.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch