[PATCH 0/2] Enhancements to notmuch-hello search history

[PATCH 0/2] Enhancements to notmuch-hello search history

[Servilio Afre Puentes]

Hi,

Two patches that enhance the notmuch-hello search history UI. Though
minor I find them very helpful.

Servilio Afre Puentes (2):
  emacs: hello: ask confirmation for clearing recent searches
  emacs: hello: allow deleting individual searches in the history

 emacs/notmuch-hello.el | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

-- 
1.8.2.1

[PATCH 1/2] emacs: hello: ask confirmation for clearing recent searches

[Servilio Afre Puentes]

The button to clear the recent searches in notmuch-hello is easy to
press accidentally while moving around the, clearing potentially
useful searches with no way of recovering them.
---
 emacs/notmuch-hello.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index c1c6f4b..aa063b7 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -599,8 +599,9 @@ Complete list of currently available key bindings:
     (widget-insert "Recent searches: ")
     (widget-create 'push-button
 		   :notify (lambda (&rest ignore)
-			     (setq notmuch-search-history nil)
-			     (notmuch-hello-update))
+			     (when (yes-or-no-p "Are you sure you want to clear the searches? ")
+			       (setq notmuch-search-history nil)
+			       (notmuch-hello-update)))
 		   "clear")
     (widget-insert "\n\n")
     (let ((start (point)))
-- 
1.8.2.1

[PATCH 2/2] emacs: hello: allow deleting individual searches in the history

[Servilio Afre Puentes]

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 («y» or «n») is made before taking action.
---
 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)))
 
+(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)
+    ))
+
 (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))
-- 
1.8.2.1

Re: [PATCH 2/2] emacs: hello: allow deleting individual searches in the history

[Mark Walters]

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 («y» or «n») is made before taking action.
> ---
>  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)))
>  
> +(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))
> -- 
> 1.8.2.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 0/2] Enhancements to notmuch-hello search history

[Jani Nikula]

On Fri, 03 May 2013, Servilio Afre Puentes <servilio@gmail.com> wrote:
> Two patches that enhance the notmuch-hello search history UI. Though
> minor I find them very helpful.

Both seem to work as advertised; I did not look at the code much. A
minor bikeshed is that I think y-or-n-p would suffice in patch 1/2.

BR,
Jani.

Re: [PATCH 0/2] Enhancements to notmuch-hello search history

[Servilio Afre Puentes]

Jani Nikula <jani@nikula.org> writes:

> On Fri, 03 May 2013, Servilio Afre Puentes <servilio@gmail.com> wrote:
>> Two patches that enhance the notmuch-hello search history UI. Though
>> minor I find them very helpful.
>
> Both seem to work as advertised; I did not look at the code much.  A
> minor bikeshed is that I think y-or-n-p would suffice in patch 1/2.

I know, but as it clears the whole history I decided to go with a
confirmation that required more attention to diminish the chance of
accident, and the action is taken not so oftenly (at least for me, and
my guess is that it is the same for most) so it stills balances out
positively in the usability.

Servilio

Re: [PATCH 0/2] Enhancements to notmuch-hello search history

[Tomi Ollila]

On Sat, May 04 2013, Servilio Afre Puentes <servilio@gmail.com> wrote:

> Jani Nikula <jani@nikula.org> writes:
>
>> On Fri, 03 May 2013, Servilio Afre Puentes <servilio@gmail.com> wrote:
>>> Two patches that enhance the notmuch-hello search history UI. Though
>>> minor I find them very helpful.
>>
>> Both seem to work as advertised; I did not look at the code much.  A
>> minor bikeshed is that I think y-or-n-p would suffice in patch 1/2.
>
> I know, but as it clears the whole history I decided to go with a
> confirmation that required more attention to diminish the chance of
> accident, and the action is taken not so oftenly (at least for me, and
> my guess is that it is the same for most) so it stills balances out
> positively in the usability.

I tested the patch and it works as advertised.

I agree with Jani that y-or-n-p would suffice for clearing all
recent searches (but yes-or-no-p is not so bad as I originally thought
as C-g can be used to stop processing (typoing 'no' is too easy ;/). 

If we could use "undo" feature to restore last clear/del then I'd agree
100% with Mark (i.e. no y-or-n-p for [del]).

In any case fix at least the style issue Mark mentioned -- any of the
yes/no queries will eventually be fine with me (as I don't know without
experience what would be the best (for me) after all).

> Servilio

Tomi

Re: [PATCH 0/2] Enhancements to notmuch-hello search history

[Jani Nikula]

On Sun, 05 May 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> I agree with Jani that y-or-n-p would suffice for clearing all
> recent searches (but yes-or-no-p is not so bad as I originally thought
> as C-g can be used to stop processing (typoing 'no' is too easy ;/). 
>
> If we could use "undo" feature to restore last clear/del then I'd agree
> 100% with Mark (i.e. no y-or-n-p for [del]).
>
> In any case fix at least the style issue Mark mentioned -- any of the
> yes/no queries will eventually be fine with me (as I don't know without
> experience what would be the best (for me) after all).

Servilio, please switch to y-or-n-p in patch 1 and fix the style issue
in patch 2, and I think we're happy on the average. :)

Jani.