From: Mark Walters <markwalters1009@gmail.com>
To: Servilio Afre Puentes <servilio@gmail.com>,
	notmuch <notmuch@notmuchmail.org>
Subject: Re: [PATCH 2/2] emacs: hello: allow deleting individual searches in
	the history
In-Reply-To: <87sj24kksc.fsf@goose.CSU.McMaster.CA>
References: <87y5bwkml2.fsf@goose.CSU.McMaster.CA>
	<87sj24kksc.fsf@goose.CSU.McMaster.CA>
User-Agent: Notmuch/0.14+255~gff3cc55 (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Sun, 05 May 2013 00:31:18 +0100
Message-ID: <87zjwafhkp.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: list


These patches both look good to me (and the lisp all looks fine) modulo
a trivial style comment and a little bikeshedding.

For the bikeshedding I would prefer that the first patch was a y-or-n-p
(as Jani suggested) and that the second didn't query at all (the
dataloss potential from deleting a single search from the history is
pretty small)


On Fri, 03 May 2013, Servilio Afre Puentes <servilio@gmail.com> wrote:
> This commit adds an extra button at the end of the search entries that
> allows deleting that individual search from the history. A short
> confirmation (=C2=ABy=C2=BB or =C2=ABn=C2=BB) is made before taking actio=
n.
> ---
>  emacs/notmuch-hello.el | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index aa063b7..9fbbdc9 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -286,6 +286,16 @@ afterwards.")
>      (message "Saved '%s' as '%s'." search name)
>      (notmuch-hello-update)))
>=20=20
> +(defun notmuch-hello-delete-search-from-history (widget)
> +  (interactive)
> +  (let ((search (widget-value
> +		 (symbol-value
> +		  (widget-get widget :notmuch-saved-search-widget)))))
> +    (setq notmuch-search-history (delete search
> +					 notmuch-search-history))
> +    (notmuch-hello-update)
> +    ))
> +

Trivial formatting/style point: notmuch goes for putting all the braces
after the final function.

Best wishes

Mark

>  (defun notmuch-hello-longest-label (searches-alist)
>    (or (loop for elem in searches-alist
>  	    maximize (length (car elem)))
> @@ -624,7 +634,12 @@ Complete list of currently available key bindings:
>  						;; `[save]' button. 6
>  						;; for the `[save]'
>  						;; button.
> -						1 6))
> +						1 6
> +						;; 1 for the space
> +						;; before the `[del]'
> +						;; button. 5 for the
> +						;; `[del]' button.
> +						1 5))
>  				  :action (lambda (widget &rest ignore)
>  					    (notmuch-hello-search (widget-value widget)))
>  				  search))
> @@ -633,7 +648,14 @@ Complete list of currently available key bindings:
>  			     :notify (lambda (widget &rest ignore)
>  				       (notmuch-hello-add-saved-search widget))
>  			     :notmuch-saved-search-widget widget-symbol
> -			     "save"))
> +			     "save")
> +	      (widget-insert " ")
> +	      (widget-create 'push-button
> +			     :notify (lambda (widget &rest ignore)
> +				       (when (y-or-n-p "Are you sure you want to delete this search?=
 ")
> +					 (notmuch-hello-delete-search-from-history widget)))
> +			     :notmuch-saved-search-widget widget-symbol
> +			     "del"))
>  	    (widget-insert "\n"))
>        (indent-rigidly start (point) notmuch-hello-indent))
>      nil))
> --=20
> 1.8.2.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch