From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Jameson Graef Rollins <jrollins@finestructure.net>,
	Notmuch Mail <notmuch@notmuchmail.org>
Subject: Re: [PATCH 3/6] emacs: modify notmuch-search-tag to not prompt if
	tags provided as argument
In-Reply-To: <1333354853-25729-4-git-send-email-jrollins@finestructure.net>
References: <1333354853-25729-1-git-send-email-jrollins@finestructure.net>
	<1333354853-25729-2-git-send-email-jrollins@finestructure.net>
	<1333354853-25729-3-git-send-email-jrollins@finestructure.net>
	<1333354853-25729-4-git-send-email-jrollins@finestructure.net>User-Agent:
	Notmuch/0.12+70~g06ecbeb (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Mon, 02 Apr 2012 12:44:21 +0400
Message-ID: <874nt2ikuy.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

Jameson Graef Rollins <jrollins@finestructure.net> writes:

> If the argument is a full string or a list, the function will assume
> this is a tag string or list and will not prompt the user.  If the
> argument is nil or the exact strings "-" or "+" then the user will be
> prompted.
>
> The function doc is updated accordingly.
> ---
>  emacs/notmuch.el |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 0ab8fc2..3b78584 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -646,13 +646,21 @@ added or removed for all threads in the region from BEG to END."
>  	   (notmuch-update-tags (notmuch-search-get-tags) tag-changes))
>  	  (forward-line))))))
>  
> -(defun notmuch-search-tag (&optional initial-input)
> -  "Change tags for the currently selected thread or region."
> +(defun notmuch-search-tag (&optional tags)
> +  "Change tags for the currently selected thread or region.
> +
> +If TAGS is a string or list it will be interpreted as tags to
> +apply to the selected messages.  If TAGS is nil or either of the
> +strings `-' or `+' the user will be prompted to enter tags (with
> +tab completion)."

The TAGS argument name may be confusing.  Other tagging functions tend
to use TAG-CHANGES for these.  Can you please change the argument name
here for consistency?

>    (interactive)
>    (let* ((beg (if (region-active-p) (region-beginning) (point)))
>  	 (end (if (region-active-p) (region-end) (point)))
> -	 (search-string (notmuch-search-find-thread-id-region-search beg end))
> -	 (tags (notmuch-read-tag-changes initial-input search-string)))
> +	 (search-string (notmuch-search-find-thread-id-region-search beg end)))
> +    (if (string-or-null-p tags)
> +	(if (or (string= tags "-") (string= tags "+") (eq tags nil))

Should we add a check for an empty string?

Please use `null'.

> +	    (setq tags (notmuch-read-tag-changes tags search-string))
> +	  (setq tags (list tags))))

Should we change `notmuch-tag' to accept strings instead of handling it
here?

Regards,
  Dmitry

>      (apply 'notmuch-search-tag-region beg end tags)))
>  
>  (defun notmuch-search-add-tag ()
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch